Skip to content

E2E test stabilization, and upgrade to latest Cypress v9 and axe-core#1898

Merged
tdonohue merged 3 commits intoDSpace:mainfrom
tdonohue:upgrade_latest_cypress_9
Nov 8, 2022
Merged

E2E test stabilization, and upgrade to latest Cypress v9 and axe-core#1898
tdonohue merged 3 commits intoDSpace:mainfrom
tdonohue:upgrade_latest_cypress_9

Conversation

@tdonohue
Copy link
Copy Markdown
Member

@tdonohue tdonohue commented Oct 12, 2022

Description

Simple PR to attempt to stabilize some randomly failing e2e tests. Based on what I've seen in recent CI runs, it looks like there are three main random failures:

  • Random authentication errors (sometimes the auth fails... I've attempted to fix this in this PR)
  • Random accessibility testing failures (Not sure our code is at fault. Appears to be a possible bug in Cypress or Axe. So, I upgraded to latest axe-core and cypress to see if that helps)
  • Random page not loading failures. Uncertain of the root cause at this time. May be a Cypress issue?

This PR does the following:

  • Upgrade us to the latest version of Cypress v9 and axe-core.
    • (NOTE: Cypress v10 is also now available, but that's a major upgrade requiring refactoring of existing e2e tests. So, that may require much more work to complete & may need to be scheduled for a future date.)
  • Change our authentication strategy in e2e tests to use the login form in the UI. Previously we were using requests directly to the backend REST API to authenticate -- while that approach is likely faster, it seems to sometimes hit random failures (possibly cause of an out-of-sync CSRF token?). It's likely more stable to use our login form at all times.

Instructions for Reviewers

  • Verify e2e tests pass & seem more stable.
  • Check code.

@tdonohue tdonohue added bug testing framework Related specifically to Unit or Integration (e2e) Tests labels Oct 12, 2022
@tdonohue tdonohue self-assigned this Oct 12, 2022
@tdonohue tdonohue changed the title Update to latest Cypress v9 E2E test stabilization, and upgrade to latest Cypress v9 and axe-core Oct 13, 2022
@tdonohue
Copy link
Copy Markdown
Member Author

This is ready for review now. While it hasn't fully stabilized all tests (After restarting tests 5 times, I did finally hit some random Axe accessibility check failures), it seems better than previously.

@tdonohue tdonohue added the 1 APPROVAL pull request only requires a single approval to merge label Oct 14, 2022
@tdonohue
Copy link
Copy Markdown
Member Author

@artlowel : This might be one you could quickly review. From what I've seen, these small changes make the e2e tests more stable, but there still will be some random failures (they seem less frequent though)

@tdonohue tdonohue requested a review from artlowel October 20, 2022 18:44
@tdonohue tdonohue added this to the 7.5 milestone Oct 20, 2022
@tdonohue tdonohue force-pushed the upgrade_latest_cypress_9 branch from e6c4c5d to 11154d2 Compare October 27, 2022 19:16
@tdonohue tdonohue requested a review from atarix83 November 3, 2022 15:36
Copy link
Copy Markdown
Contributor

@atarix83 atarix83 left a comment

Choose a reason for hiding this comment

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

thanks @tdonohue

it seems to be better, but we can't be 100% sure it resolves the random errors definitely.
I think we need to keep monitored for upcoming builds after the merge and plan to migrate to new cypress v10

@tdonohue
Copy link
Copy Markdown
Member Author

tdonohue commented Nov 8, 2022

Agreed @atarix83. I don't think this is going to fix the random failures 100% of the time, but I find the random failures are less frequent with this PR in place. I also agree that an upgrade to Cypress v10 is needed at some point...however, that's a major upgrade (I attempted it locally & found it to be quite complex & wasn't able to get it working properly -- I think v10 still is a bit buggy).

In any case, merging this immediately & we'll see if this has a decent impact or if we need to prioritize a Cypress v10 upgrade sooner.

@tdonohue tdonohue merged commit 340cc0a into DSpace:main Nov 8, 2022
@tdonohue tdonohue deleted the upgrade_latest_cypress_9 branch November 8, 2022 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1 APPROVAL pull request only requires a single approval to merge bug testing framework Related specifically to Unit or Integration (e2e) Tests

Projects

No open projects
Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

2 participants