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

Adjusting e2e tests + support code for Phase 1 store refactor #330

Merged

Conversation

jarmoza
Copy link
Contributor

@jarmoza jarmoza commented Jan 30, 2023

This PR addresses #309, #310, and #311.

309 - refactor page test setup

  1. Refactored loadTestDataIntoStore to account for new store actions for the data table and dictionary as well as the now-required order for file selection (data table -> data dictionary).
  2. Added new support function commitToNuxtStore to address the need to set the current page when programmatically routing between annotation tool pages. More on that below.
  3. Removed ability to only check next page button and not navbar from assertNextPageAccess as it is no longer used in any e2e tests. (With the refactor, the annotation page will no longer have UI elements with their own 'next' buttons – where this test functionality was used.)

310 - adjust index page tests

  1. The second e2e test for the index page now just checks to make sure that the data dictionary file selection is disabled while a data table has not yet been selected.
  2. Some light reformatting and test string adjustments.

311 - adjust categorization page tests

  1. All category tool group tests have been removed from the e2e tests for the categorization page.
  2. A multi-category linking test has been added to ensure next page access is maintained after a second non-Subject ID categorization has been done. (Current 'Subject ID' categorization is a special case that grants access to the next page.)
  3. Store mutation setCurrentPage is now called after routing to the categorization page has been done. Without this direct call, the tool believed the current page to still be the home page after the first call to cy.visit('/') and errored in showing what looked to be the enabled next page button for the home page. (The currentPage store field is now only set when a navbar item or next page button is clicked on, so mutation setCurrentPage needed to be called as part of a programmatic navigation.)
  4. Some light reformatting and test string adjustments.

@jarmoza jarmoza requested a review from surchs January 30, 2023 20:55
@jarmoza jarmoza added categorization page landing page maint:refactor Simplifying or restructuring existing code or documentation. maint:coverage Test coverage improvements that were not included in feature prioritization labels Jan 30, 2023
Copy link
Contributor

@surchs surchs 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 @jarmoza! Very cool moment to see the app actually come back to life after all this refactoring 🎉! All looks good to me, ready to merge. I left two comments more out of curiosity. Depending on your answer for the index-page test one, I would maybe make the comment for the "click data-dictionary button" step a bit clearer regarding ("why cannot we just assert that this label-button thing is not clickable here")

edit:
Oh, forgot an important point: now that the e2e tests can (at least in principle) pass again, should we exclude the annotation page e2e tests until they are reasonably passing? The goal would be to have from now and going forward fully green tests again!

cypress/e2e/page/index-pagetests.cy.js Show resolved Hide resolved
cypress/support/commands.js Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
categorization page landing page maint:coverage Test coverage improvements that were not included in feature prioritization maint:refactor Simplifying or restructuring existing code or documentation.
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

None yet

2 participants