Skip to content

test: create tests for datasets backpages (#4080) #4089

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

Jump to bottom
Merged
merged 10 commits into from
Sep 10, 2024
Merged

test: create tests for datasets backpages (#4080) #4089

merged 10 commits into from
Sep 10, 2024

Conversation

mrtopsyt
Copy link
Contributor

@mrtopsyt mrtopsyt commented Aug 1, 2024 •
edited
Loading

Ticket

Closes #4080 .

Reviewers

@MillenniumFalconMechanic .

Changes

  • Added the following backpage tests:
    • Check that information in the "Dataset details" tab matches relevant fields in the table
    • Check access control works properly -i.e. restricted datasets do not allow for export and unrestricted datasets do
      • Currently disabled as export is disabled
    • Smoke test for "Request link" button
      • Check boxes
      • Press "Request link" button
      • Press resulting button to go to Terra
      • Check that url begins "anvil.terra.bio"
      • Currently disabled as export is disabled
    • Added interfaces and constants in anvilTabs.ts and testInterfaces.ts so that there were no hardcoded constants in the new tests

Please do not merge before filter tests (#4071)

@github-actions github-actions bot added the canary Done by the Clever Canary team label Aug 1, 2024
@mrtopsyt mrtopsyt force-pushed the jonah/4080-create-tests-for-datasets-backpages branch from 4abe3b4 to e430182 Compare August 23, 2024 21:29
@mrtopsyt mrtopsyt changed the title Jonah/4080 create tests for datasets backpages test: create tests for datasets backpages (#4080) Aug 23, 2024
MillenniumFalconMechanic
MillenniumFalconMechanic requested changes Aug 27, 2024
View reviewed changes
Copy link
Contributor

@MillenniumFalconMechanic MillenniumFalconMechanic left a comment

Choose a reason for hiding this comment

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

Looks good, thanks Jonah! I have requested a couple of changes.

explorer/e2e/anvil/anvil-tabs-buttons.spec.ts Outdated
await page.goto(anvilTabs.datasets.url);
await testTab(page, tab);
await testTab(page, anvilTabs.datasets);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be anvilTabs.biosamples?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it should! Thanks for catching that!

explorer/e2e/anvil/anvil-tabs.ts Outdated
];
const anvilDatasetsSelectableColumns = [
{
name: "Phenotypic Sex",
Copy link
Contributor

Choose a reason for hiding this comment

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

Thoughts on adding constants for these column names (e.g. could we can get reuse here and here)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do, although since this doesn't affect test functionality it might make more sense to do it under #4078 since it would affect all tests! Would that work?

Copy link
Contributor

Choose a reason for hiding this comment

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

Would you be able to go ahead and update these in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do!

explorer/e2e/testFunctions.ts Outdated
@@ -522,4 +541,255 @@ export async function testClearAll(
await page.locator("body").click();
}
}

const getRowLocatorByAccess = (page: Page, access: string): Locator =>
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you be able to add TSDocs for the functions added in this PR?

explorer/e2e/testFunctions.ts Outdated
Comment on lines 712 to 713
columnDescription != undefined &&
columnDescription.pluralizedLabel != undefined &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Checking in on the type coercion here (!= undefined): are you explicitly checking for undefined or null? Let's use !columnDescription && !columnDescription.pluralizedLabel if you're just checking for a falsy case, or !== undefined if you're looking specifically for undefined.

Copy link
Contributor Author

@mrtopsyt mrtopsyt Aug 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Will fix that - still not used to JS type coercion but I'll review these. Thanks for catching it!

explorer/e2e/testFunctions.ts Outdated
header?.correspondingColumn?.name === columnHeaderName
)?.name;
if (correspondingHeaderName == null) {
// Fail the test, because this means there is an incorrect configuraiton in the tab definition
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Fail the test, because this means there is an incorrect configuraiton in the tab definition
// Fail the test, because this means there is an incorrect configuration in the tab definition

@mrtopsyt mrtopsyt force-pushed the jonah/4080-create-tests-for-datasets-backpages branch from 8c9475b to 3853267 Compare August 31, 2024 19:27
@mrtopsyt mrtopsyt force-pushed the jonah/4080-create-tests-for-datasets-backpages branch from 3853267 to 894a237 Compare September 3, 2024 23:57
@mrtopsyt mrtopsyt force-pushed the jonah/4080-create-tests-for-datasets-backpages branch from 31ee683 to a770114 Compare September 6, 2024 22:02
MillenniumFalconMechanic
MillenniumFalconMechanic approved these changes Sep 10, 2024
View reviewed changes
Copy link
Contributor

@MillenniumFalconMechanic MillenniumFalconMechanic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Jonah!

@mrtopsyt mrtopsyt merged commit 5baba61 into main Sep 10, 2024
4 checks passed
@mrtopsyt mrtopsyt deleted the jonah/4080-create-tests-for-datasets-backpages branch September 10, 2024 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MillenniumFalconMechanic MillenniumFalconMechanic MillenniumFalconMechanic approved these changes

Assignees
No one assigned
Labels
canary Done by the Clever Canary team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create tests for datasets backpages
2 participants
@mrtopsyt @MillenniumFalconMechanic