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

chore: update system tests and prep for react18 harness removal/upstream merge for component testing #30614

Merged

Conversation

AtofStryker
Copy link
Contributor

@AtofStryker AtofStryker commented Nov 14, 2024

  • Closes N/A

Additional details

The goal of this PR is to get our system tests up to date with the latest technologies that we support. This means:

  • moving more projects from webpack/webpack-dev-server from v4 to v5 and related loaders/dependencies
  • updating typescript from v4 to v5 is most tests (except ts 4 explicit tests)
  • updating react system tests from 16x/17x to 18x and using the cypress/react18 test harness as a precursor to breaking: remove support for React 16 and 17 for Cypress Component Testing. Additionally, remove the cypress/react18 testing harness and merge it upstream with cypress/react #30590 (react17 system tests are removed here).
  • trying to get closer alignment in our system tests package.json
  • removing the webpack-preprocessor-awesome-typescript-loader tests as it isn't actively maintained and ts-loader is now the used loader by most
  • removing the react-app-webpack-5-unconfigured app and replacing with react18-webpack-unconfigured as it is a lot smaller and easier to maintain (not the ejected create-react-app)
  • issue-25951-next-app is removed as this is tested with next versions over 13 passively. We currently support 14+
  • the module-api snapshots needed to be updated with updating snap-shot-it 2 major versions, so now the snapshots live in a single file

to make the system tests on webpack more deterministic, I changed the stat output to be errors only so we don't have a lot of drift when new modules or times exist in the snapshot

Steps to test

How has the user experience changed?

PR Tasks

@AtofStryker AtofStryker self-assigned this Nov 14, 2024
@AtofStryker AtofStryker added the type: chore Work is required w/ no deliverable to end user label Nov 14, 2024
Copy link

cypress bot commented Nov 14, 2024

cypress    Run #58413

Run Properties:  status check passed Passed #58413  •  git commit cbdb33a5bb: chore: update system tests to use react 18 and install latest dependencies for a...
Project cypress
Branch Review chore/update-tests-prep-react-18-ct-update
Run status status check passed Passed #58413
Run duration 17m 44s
Commit git commit cbdb33a5bb: chore: update system tests to use react 18 and install latest dependencies for a...
Committer AtofStryker
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 4
Tests that did not run due to a developer annotating a test with .skip  Pending 1326
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 29318
View all changes introduced in this branch ↗︎
UI Coverage  45.79%
  Untested elements 188  
  Tested elements 163  
Accessibility  92.56%
  Failed rules  3 critical   8 serious   2 moderate   2 minor
  Failed elements 892  

…ncies for applicable system tests. use react18 harness before removal [run ci]
@AtofStryker AtofStryker force-pushed the chore/update-tests-prep-react-18-ct-update branch from 04b374f to cbdb33a Compare November 14, 2024 02:14
@@ -1789,7 +1789,7 @@ jobs:
PLATFORM: linux
machine:
# using `machine` gives us a Linux VM that can run Docker
image: ubuntu-2004:202111-02
image: ubuntu-2004:2024.05.1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

needs to be updated since find-up update in the module-api system test is now on version 7 and cannot be installed with node 16. ubuntu-2004:202111-02 ships with node 16, as ubuntu-2004:2024.05.1 ships with node 20.13.0

@@ -42,7 +42,7 @@
"src/**/*.js"
],
"engines": {
"node": ">=8"
"node": ">=18"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just a missed detail. Cypress requires node 18 and up so this is a non breaking change

line: 26,
column: 8,
line: 19,
isPreprocessorWithTypescript: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is needed here since we are having stack codeframe/stacktraces now transpiled from typescript 4 to 5, which is slightly different than what is expected in the javascript context

@@ -70,6 +71,8 @@ export const verify = (title, ctx, options) => {
// code frames will show this as the 1st line
if (isCyOrigin) {
cy.get('.test-err-code-frame pre span').should('include.text', `('${title}',,function()`)
} else if (isPreprocessorWithTypescript) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

has to do with using typescript in some of these on ts version 5 vs 5, which means the transpilation is slightly different. This is related to #29614

@@ -616,6 +625,23 @@ When Cypress detects uncaught errors originating from your test code it will aut
Cypress could not associate this error to any specific test.

We dynamically generated a new test to display this failure.
Error: The following error originated from your test code, not from Cypress.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this error showing up twice in the output?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the update to babel-loader from from 8 to 9. We usually append the error in the stack output if we can find it, and I think the enhancement there is making it show up

Copy link
Collaborator

Choose a reason for hiding this comment

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

Will that look weird to the user to have it show up twice? Or is this just a system test nuance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't believe so since this is our default behavior if we can discover a message. My guess is previously there wasn't a message attached with the error and now there is, so we have something to print

@AtofStryker AtofStryker merged commit 53b24b1 into release/14.0.0 Nov 14, 2024
83 of 85 checks passed
@AtofStryker AtofStryker deleted the chore/update-tests-prep-react-18-ct-update branch November 14, 2024 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: chore Work is required w/ no deliverable to end user
Projects
None yet
2 participants