Skip to content
This repository was archived by the owner on Apr 29, 2025. It is now read-only.

AG-1435#1304

Merged
hallieswan merged 6 commits intoSage-Bionetworks:developfrom
hallieswan:AG-1435
May 14, 2024
Merged

AG-1435#1304
hallieswan merged 6 commits intoSage-Bionetworks:developfrom
hallieswan:AG-1435

Conversation

@hallieswan
Copy link
Copy Markdown
Contributor

Comment thread .eslintrc.json
"ecmaVersion": "latest",
"sourceType": "module"
"sourceType": "module",
"project": ["./tsconfig.base.json"]
Copy link
Copy Markdown
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.

Comment thread package-lock.json
"@babel/plugin-proposal-decorators": "^7.18.10",
"@ngtools/webpack": "^13.3.9",
"@playwright/test": "^1.39.0",
"@playwright/test": "^1.43.1",
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Bump playwright version

Comment on lines +75 to 77
// https://github.com/angular/angular/issues/45202
// eslint-disable-next-line @typescript-eslint/no-floating-promises
router.navigate(['/about']);
Copy link
Copy Markdown
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.

Comment on lines +57 to +58
beforeEach(waitForAsync(async () => {
await TestBed.configureTestingModule({
Copy link
Copy Markdown
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

Comment thread tsconfig.base.json
Comment on lines +2 to +28
"compileOnSave": false,
"compilerOptions": {
"baseUrl": "./src",
"outDir": "./dist/out-tsc",
"forceConsistentCasingInFileNames": true,
"strict": true,
"noImplicitOverride": true,
"noPropertyAccessFromIndexSignature": false,
"noImplicitReturns": true,
"noFallthroughCasesInSwitch": true,
"sourceMap": true,
"declaration": false,
"downlevelIteration": true,
"experimentalDecorators": true,
"moduleResolution": "node",
"importHelpers": true,
"target": "es2017",
"module": "es2020",
"lib": ["es2020", "dom"],
"allowSyntheticDefaultImports": true
},
"angularCompilerOptions": {
"enableI18nLegacyMessageIdFormat": false,
"strictInjectionParameters": true,
"strictInputAccessModifiers": true,
"strictTemplates": true
},
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Moved from tsconfig.json

Comment thread tsconfig.base.json
Comment on lines +29 to +36
"include": [
"./src/**/*.ts",
"./src/**/*.html",
"./src/**/*.scss",
"./tests/**.ts",
"./tests/**/**.ts",
"playwright.config.ts"
]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Specifies which files should be linted

Comment thread tsconfig.json
"strictInputAccessModifiers": true,
"strictTemplates": true
},
"extends": "./tsconfig.base.json",
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Extend the base tsconfig, so that an exclude block can still be used when bundling via webpack

test('invalid gene results in a 404 redirect', async ({ page }) => {
// go to invalid ENSG page
await page.goto('/genes/ENSG00000000000');
await waitForSpinnerNotVisible(page);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Waiting for loader to be removed reduces flakiness when page takes a long time to load


// expect div for page not found content to be visible
expect(page.locator('.page-not-found')).toBeVisible();
await expect(page.locator('.page-not-found')).toBeVisible();
Copy link
Copy Markdown
Contributor Author

@hallieswan hallieswan May 2, 2024

Choose a reason for hiding this comment

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

Fix warning from @typescript-eslint/no-floating-promises -- toBeVisible is an auto-retrying assertion, so should be awaited

Comment thread tests/gene-comparison-tool-pinning-cache.spec.ts Outdated
@hallieswan hallieswan marked this pull request as ready for review May 2, 2024 21:30
@hallieswan hallieswan requested a review from sagely1 May 2, 2024 21:30
<div *ngIf="genesTable.filteredValue?.length">
<button
class="pin-all-button"
[disabled]="getPinDisabledStatus()"
Copy link
Copy Markdown
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.

Comment on lines +143 to +148
test.fail('when RNA url includes proteins, the related gene is pinned', {
annotation: {
type: 'fail',
description: 'Since AG-1425, only genes will be pinned from RNA url'
}
},
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Expect this test to fail -- since AG-1425, only genes will be pinned from an RNA url. ensembl id + uniprot id will be dropped.

Copy link
Copy Markdown
Contributor

@sagely1 sagely1 left a comment

Choose a reason for hiding this comment

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

Looks great!

@hallieswan hallieswan merged commit aea77e0 into Sage-Bionetworks:develop May 14, 2024
@hallieswan hallieswan deleted the AG-1435 branch May 14, 2024 16:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants