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.

Sign up for GitHub

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

AG-1435 #1304

Merged
merged 6 commits into from
May 14, 2024
Merged

AG-1435 #1304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1a74c6b
AG-1435: bump playwright version
hallieswan Apr 29, 2024
cc9c078
AG-1435: lint e2e tests as described in playwright best practices
hallieswan Apr 30, 2024
1e5ade8
AG-1435: add e2e tests for gct pinning
hallieswan May 2, 2024
feb334c
AG-1435: keep warnings for floating promises that may need to be fixe…
hallieswan May 2, 2024
2293d4a
AG-1435: improve test naming
hallieswan May 2, 2024
694d9de
AG-1435: updates based on AG-1425
hallieswan May 13, 2024
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
6 changes: 4 additions & 2 deletions .eslintrc.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
"parser": "@typescript-eslint/parser",
"parserOptions": {
"ecmaVersion": "latest",
"sourceType": "module"
"sourceType": "module",
"project": ["./tsconfig.base.json"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Setting the @typescript-eslint/no-floating-promises rule causes this error: You have used a rule which requires parserServices to be generated. You must therefore provide a value for the "parserOptions.project" property for @typescript-eslint/parser., so specify a project.

},
"plugins": ["@typescript-eslint"],
"rules": {
Expand All @@ -23,6 +24,7 @@
"@typescript-eslint/ban-ts-comment": "off",
"@typescript-eslint/no-empty-interface": "off",
"@typescript-eslint/no-empty-function": "off",
"@typescript-eslint/no-this-alias": "off"
"@typescript-eslint/no-this-alias": "off",
"@typescript-eslint/no-floating-promises": "warn"
}
}
46 changes: 23 additions & 23 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@
"mongo:start:windows": "sh ./scripts/start-mongo.sh 'C:\\data\\db'",
"data:local": "npm run clean:data && sh ./scripts/get-data-local.sh",
"e2e": "playwright test --trace on",
"e2e:ui": "playwright test --ui"
"e2e:ui": "playwright test --ui",
"e2e:update": "npm install -D @playwright/test@latest; npx playwright install --with-deps; npx playwright --version"
},
"pre-commit": [
"test"
Expand Down Expand Up @@ -94,7 +95,7 @@
"@angularclass/hmr": "^3.0.0",
"@babel/plugin-proposal-decorators": "^7.18.10",
"@ngtools/webpack": "^13.3.9",
"@playwright/test": "^1.39.0",
"@playwright/test": "^1.43.1",
"@types/aws-param-store": "^2.1.2",
"@types/browser-update": "^3.3.0",
"@types/compression": "^1.7.2",
Expand Down
2 changes: 2 additions & 0 deletions src/app/core/components/footer/footer.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ describe('Component: Footer', () => {
const link = element.querySelector('.footer-logo a') as HTMLElement;
expect(link).toBeTruthy();

// https://github.com/angular/angular/issues/45202
// eslint-disable-next-line @typescript-eslint/no-floating-promises
router.navigate(['/about']);
Comment on lines +75 to 77
Copy link
Contributor Author

Choose a reason for hiding this comment

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

router.navigate returns a Promise, but is not awaited in Agora or in the Angular docs. Per the referenced link, it's common practice to ignore the Promise, so disable the lint rule for these cases.

tick();
expect(location.path()).toBe('/about');
Expand Down
2 changes: 2 additions & 0 deletions src/app/core/components/header/header.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ describe('Component: Header', () => {
const link = element.querySelector('.header-logo a') as HTMLElement;
expect(link).toBeTruthy();

// https://github.com/angular/angular/issues/45202
// eslint-disable-next-line @typescript-eslint/no-floating-promises
router.navigate(['/nominated-targets']);
tick();
expect(location.path()).toBe('/nominated-targets');
Expand Down
4 changes: 2 additions & 2 deletions src/app/features/charts/components/median-barchart/median-barchart.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ describe('Component: BarChart - Median', () => {
let component: MedianBarChartComponent;
let element: HTMLElement;

beforeEach(waitForAsync(() => {
TestBed.configureTestingModule({
beforeEach(waitForAsync(async () => {
await TestBed.configureTestingModule({
Comment on lines +57 to +58
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Await promise, so there is not a warning from newly enabled @typescript-eslint/no-floating-promises rule

declarations: [MedianBarChartComponent],
imports: [RouterTestingModule],
providers: [HelperService],
Expand Down
2 changes: 2 additions & 0 deletions src/app/features/genes/components/gene-comparison-tool/gene-comparison-tool.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,7 @@ <h1 class="gct-heading h2">Gene Comparison Tool</h1>
<div *ngIf="genesTable.filteredValue?.length">
<button
class="pin-all-button"
[disabled]="getPinDisabledStatus()"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Set the disabled property, rather than just a 'disabled' class, so that Playwright can confirm that the pin gene button is disabled when pinned gene limit has been reached.

[ngClass]="{
disabled: getPinDisabledStatus()
}"
Expand Down Expand Up @@ -471,6 +472,7 @@ <h1 class="gct-heading h2">Gene Comparison Tool</h1>
<div>
<button
(click)="onPinGeneClick(gene)"
[disabled]="getPinDisabledStatus()"
[ngClass]="{
disabled: getPinDisabledStatus()
}"
Expand Down
4 changes: 2 additions & 2 deletions ...app/features/genes/components/gene-comparison-tool/gene-comparison-tool.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@
let element: HTMLElement;
let route: ActivatedRoute;

beforeEach(fakeAsync(() => {
TestBed.configureTestingModule({
beforeEach(fakeAsync(async () => {
await TestBed.configureTestingModule({
declarations: [
GeneComparisonToolComponent,
GeneComparisonToolDetailsPanelComponent,
Expand Down Expand Up @@ -431,7 +431,7 @@
) as HTMLElement;
filterClearButton.click();

fixture.whenStable().then(() => {

Check warning on line 434 in src/app/features/genes/components/gene-comparison-tool/gene-comparison-tool.component.spec.ts

View workflow job for this annotation

GitHub Actions / build (16.x) 

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
fixture.detectChanges();
expectSignificanceThresholdIsNotApplied();
});
Expand All @@ -449,7 +449,7 @@
) as HTMLElement;
clearAllButton.click();

fixture.whenStable().then(() => {

Check warning on line 452 in src/app/features/genes/components/gene-comparison-tool/gene-comparison-tool.component.spec.ts

View workflow job for this annotation

GitHub Actions / build (16.x) 

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
fixture.detectChanges();
expectSignificanceThresholdIsNotApplied();
});
Expand All @@ -460,14 +460,14 @@
component.initData([comparisonGeneMock1, comparisonGeneMock2]);
route.queryParams = of({});

fixture.whenStable().then(() => {

Check warning on line 463 in src/app/features/genes/components/gene-comparison-tool/gene-comparison-tool.component.spec.ts

View workflow job for this annotation

GitHub Actions / build (16.x) 

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
fixture.detectChanges();
expectSignificanceThresholdIsNotApplied();

const toggle = element.querySelector(TOGGLE_CLASS) as HTMLElement;
toggle.click();

fixture.whenStable().then(() => {

Check warning on line 470 in src/app/features/genes/components/gene-comparison-tool/gene-comparison-tool.component.spec.ts

View workflow job for this annotation

GitHub Actions / build (16.x) 

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
fixture.detectChanges();
expectSignificanceThresholdIsApplied();
});
Expand Down
2 changes: 2 additions & 0 deletions src/app/features/genes/components/gene-details/gene-details.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ export class GeneDetailsComponent implements OnInit, AfterViewInit {
.subscribe((gene) => {
if (!gene) {
this.helperService.setLoading(false);
// https://github.com/angular/angular/issues/45202
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.router.navigateByUrl('/404-not-found', { skipLocationChange: true });
} else {
this.gene = gene;
Expand Down
2 changes: 2 additions & 0 deletions src/app/features/genes/components/gene-network/gene-network.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ export class GeneNetworkComponent implements OnInit {
}

navigateToSimilarGenes() {
// https://github.com/angular/angular/issues/45202
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.router.navigate([
'/genes/' + this._gene?.ensembl_gene_id + '/similar',
]);
Expand Down
2 changes: 2 additions & 0 deletions src/app/features/genes/components/gene-search/gene-search.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,8 @@ export class GeneSearchComponent extends Unsub implements AfterViewInit {
this.results = [];
this.showGeneResults = false;
this.searchNavigated.emit();
// https://github.com/angular/angular/issues/45202
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.router.navigate(['/genes/' + id]);
}

Expand Down
4 changes: 4 additions & 0 deletions src/app/features/genes/components/gene-similar/gene-similar.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ export class GeneSimilarComponent implements OnInit {
.subscribe((gene: Gene | null) => {
if (!gene) {
this.helperService.setLoading(false);
// https://github.com/angular/angular/issues/45202
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.router.navigateByUrl('/404-not-found', { skipLocationChange: true });
} else {
this.gene = gene;
Expand Down Expand Up @@ -144,6 +146,8 @@ export class GeneSimilarComponent implements OnInit {
navigateToGeneComparisonTool() {
const ids: string[] = this.genes.map((g: Gene) => g.ensembl_gene_id);
this.helperService.setGCTSelection(ids);
// https://github.com/angular/angular/issues/45202
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.router.navigate(['/genes/comparison']);
}
}
6 changes: 6 additions & 0 deletions src/app/features/genes/components/gene-table/gene-table.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@
}

navigateToGene(gene: any) {
// https://github.com/angular/angular/issues/45202
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.router.navigate(['/genes/' + gene.ensembl_gene_id]);
}

Expand All @@ -165,21 +167,25 @@

if (el[0]) {
if (!screenfull.isFullscreen) {
screenfull.request(el[0]);

Check warning on line 170 in src/app/features/genes/components/gene-table/gene-table.component.ts

View workflow job for this annotation

GitHub Actions / build (16.x) 

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
} else {
screenfull.exit();

Check warning on line 172 in src/app/features/genes/components/gene-table/gene-table.component.ts

View workflow job for this annotation

GitHub Actions / build (16.x) 

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
}
}
}

navigateToGeneComparisonTool() {
if (typeof this.gctLink === 'object') {
// https://github.com/angular/angular/issues/45202
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.router.navigate(['/genes/comparison'], {
queryParams: this.gctLink,
});
} else {
const ids: string[] = this._genes.map((g: Gene) => g.ensembl_gene_id);
this.helperService.setGCTSelection(ids);
// https://github.com/angular/angular/issues/45202
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.router.navigate(['/genes/comparison']);
}
}
Expand Down
Loading
Loading