Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(components/lists): add paging test harnesses #3185

Open
wants to merge 3 commits into
base: 11.x.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export class DemoDataService {
totalCount: people.length,
}).pipe(
// Simulate network latency.
delay(1000),
delay(100),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we reducing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was worried about this delay causing the sample tests I was writing to time out. 100ms is still a pretty good length of delay.

);
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<sky-paging
data-sky-id="my-paging-content"
[itemCount]="(pagedData | async)?.totalCount ?? 0"
[maxPages]="3"
[pageSize]="pageSize"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import { TestbedHarnessEnvironment } from '@angular/cdk/testing/testbed';
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { NoopAnimationsModule } from '@angular/platform-browser/animations';
import { SkyPagingHarness, SkyRepeaterHarness } from '@skyux/lists/testing';

import { DemoComponent } from './demo.component';

describe('Paging demo', () => {
async function setupTest(
options: {
dataSkyId?: string;
} = {},
): Promise<{
pagingHarness: SkyPagingHarness;
fixture: ComponentFixture<DemoComponent>;
}> {
await TestBed.configureTestingModule({
imports: [DemoComponent, NoopAnimationsModule],
}).compileComponents();

const fixture = TestBed.createComponent(DemoComponent);
const loader = TestbedHarnessEnvironment.documentRootLoader(fixture);

const pagingHarness: SkyPagingHarness = options.dataSkyId
? await loader.getHarness(
SkyPagingHarness.with({
dataSkyId: options.dataSkyId,
}),
)
: await loader.getHarness(SkyPagingHarness);

return { pagingHarness, fixture };
}

it('should set up the paging content', async () => {
const { pagingHarness, fixture } = await setupTest({
dataSkyId: 'my-paging-content',
});

await expectAsync(pagingHarness.getCurrentPage()).toBeResolvedTo(1);

const contentHarness = await (
await pagingHarness.getPagingContent()
).queryHarness(SkyRepeaterHarness);

let items = await contentHarness.getRepeaterItems();

await expectAsync(items[0].getTitleText()).toBeResolvedTo('Abed');

const controls = await pagingHarness.getPageControls();

await controls[2].clickButton();

fixture.detectChanges();
await fixture.whenStable();

await expectAsync(pagingHarness.getCurrentPage()).toBeResolvedTo(3);

items = await contentHarness.getRepeaterItems();

await expectAsync(items[0].getTitleText()).toBeResolvedTo('Leonard');

await pagingHarness.clickNextButton();

fixture.detectChanges();
await fixture.whenStable();

await expectAsync(pagingHarness.getCurrentPage()).toBeResolvedTo(4);

items = await contentHarness.getRepeaterItems();

await expectAsync(items[0].getTitleText()).toBeResolvedTo('Shirley');

await expectAsync(pagingHarness.clickNextButton()).toBeRejectedWithError(
'Could not click the next button because it is disabled.',
);
});
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a code example spec file for the basic one as well? I know we havent been being stringent about that but we should moving forward imo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't make one because the basic code example isn't being used. If we decide to change that, we can add it.

Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<sky-paging
[itemCount]="(pagedData | async)?.totalCount ?? 0"
[maxPages]="maxPages"
[pageSize]="pageSize"
pagingLabel="Paging label"
[(currentPage)]="currentPage"
(contentChange)="contentChange.next($event)"
>
<sky-paging-content>
<sky-repeater>
@for (person of (pagedData | async)?.people; track person) {
<sky-repeater-item>
<sky-repeater-item-title>
{{ person.name }}
</sky-repeater-item-title>
<sky-repeater-item-content>
<sky-description-list>
<sky-description-list-content>
<sky-description-list-term>
Formal name
</sky-description-list-term>
<sky-description-list-description>
{{ person.formal }}
</sky-description-list-description>
</sky-description-list-content>
</sky-description-list>
</sky-repeater-item-content>
</sky-repeater-item>
}
</sky-repeater>
</sky-paging-content>
</sky-paging>

<sky-paging data-sky-id="other-paging"></sky-paging>
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
import { CommonModule } from '@angular/common';
import { ChangeDetectionStrategy, Component } from '@angular/core';
import { SkyDescriptionListModule } from '@skyux/layout';
import {
SkyPagingContentChangeArgs,
SkyPagingModule,
SkyRepeaterModule,
} from '@skyux/lists';

import { Subject, of, shareReplay, switchMap } from 'rxjs';

const people = [
{
name: 'Abed',
formal: 'Mr. Nadir',
},
{
name: 'Alex',
formal: 'Mr. Osbourne',
},
{
name: 'Ben',
formal: 'Mr. Chang',
},
{
name: 'Britta',
formal: 'Ms. Perry',
},
{
name: 'Buzz',
formal: 'Mr. Hickey',
},
{
name: 'Craig',
formal: 'Mr. Pelton',
},
{
name: 'Elroy',
formal: 'Mr. Patashnik',
},
{
name: 'Garrett',
formal: 'Mr. Lambert',
},
{
name: 'Ian',
formal: 'Mr. Duncan',
},
{
name: 'Jeff',
formal: 'Mr. Winger',
},
{
name: 'Leonard',
formal: 'Mr. Rodriguez',
},
{
name: 'Neil',
formal: 'Mr. Neil',
},
{
name: 'Pierce',
formal: 'Mr. Hawthorne',
},
{
name: 'Preston',
formal: 'Mr. Koogler',
},
{
name: 'Rachel',
formal: 'Ms. Rachel',
},
{
name: 'Shirley',
formal: 'Ms. Bennett',
},
{
name: 'Todd',
formal: 'Mr. Jacobson',
},
{
name: 'Troy',
formal: 'Mr. Barnes',
},
{
name: 'Vaughn',
formal: 'Mr. Miller',
},
{
name: 'Vicki',
formal: 'Ms. Jenkins',
},
];

@Component({
standalone: true,
selector: 'test-paging-harness',
templateUrl: './paging-harness-test.component.html',
imports: [
CommonModule,
SkyDescriptionListModule,
SkyPagingModule,
SkyRepeaterModule,
],
changeDetection: ChangeDetectionStrategy.OnPush,
})
export class PagingHarnessTestComponent {
public pageSize = 5;
public maxPages = 3;
protected currentPage = 1;
protected contentChange = new Subject<SkyPagingContentChangeArgs>();

protected pagedData = this.contentChange.pipe(
switchMap((args) => {
const startIndex = (args.currentPage - 1) * this.pageSize;

args.loadingComplete();

return of({
people: people.slice(startIndex, startIndex + this.pageSize),
totalCount: people.length,
});
}),
shareReplay(1),
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { ComponentHarness, TestElement } from '@angular/cdk/testing';

/**
* Harness to interact with a page control element in tests.
*/

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since consumers dont set the page number text and only know it as the 'page number' and not 'page control text', i think we should mark this as internal and use the SkyPagingHarness to have functions to just get getActivePageNumber and clickPageNumber etc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this approach because it's not immediately clear how they should interact with the controls if they're obfuscated behind the paging harness. Do they ask to click a specific page number, which may not be in the list? Do they ask to click the Nth page control, and if so how do we enforce there are only that many controls to begin with? Giving them an array of the paging controls that exist feels very intuitive for them exploring the structure of the page to me. They get, say, 5 test harness elements, they can do a sanity check that they are paging in the way they expect, and interact with them the same way a user would.

export class SkyPageControlHarness extends ComponentHarness {
/**
* @internal
*/
public static hostSelector = 'li.sky-list-paging-link';

#getButton = this.locatorFor('button.sky-paging-btn');

/**
* Clicks the page button.
*/
public async clickButton(): Promise<void> {
const button = await this.#getButton();

if (await this.#buttonIsDisabled(button)) {
const label = await button.text();
throw new Error(
`Could not click page button ${label} because it is currently the active page.`,
);
}

await button.click();
}

/**
* Gets the page button text.
*/
public async getText(): Promise<string> {
const button = await this.#getButton();

return await button.text();
}

/**
* Whether the page button is disabled.
*/
public async isDisabled(): Promise<boolean> {
const button = await this.#getButton();
return await this.#buttonIsDisabled(button);
}

async #buttonIsDisabled(button: TestElement): Promise<boolean> {
const disabled = await button.getAttribute('disabled');
return disabled !== null;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { SkyQueryableComponentHarness } from '@skyux/core/testing';

/**
* Harness to interact with a paging content component in tests.
*/
export class SkyPagingContentHarness extends SkyQueryableComponentHarness {
/**
* @internal
*/
public static hostSelector = 'sky-paging-content';
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { SkyHarnessFilters } from '@skyux/core/testing';

/**
* A set of criteria that can be used to filter a list of `SkyPagingHarness` instances.
*/
// eslint-disable-next-line @typescript-eslint/no-empty-interface, @typescript-eslint/no-empty-object-type
export interface SkyPagingHarnessFilters extends SkyHarnessFilters {}
Loading
Loading