Bump Node to `v18.18.1`; Remove puppeteer #7274

1Copenut · 2023-10-10T19:54:20Z

Summary

PR captures a few streams of work that are related and easier to consolidate in one changeset:

Bump our EUI Docker container to use ~~node-18-18.0-slim~~ node-18-18.1-slim as the base image. This matches the current downloadable version of Node for general use. A second PR will follow promptly to update EUI to use Node ~~v18.18.0~~ v18.18.1 for daily work as well.
Remove the yarn add global cypress from our Docker image. This will make upgrading to Cypress 13 easier.
Remove the puppeteer user from our Docker image. Remove the Puppeteer script to check document pages for accessibility. We're using Cypress component tests and will roll new doc page checks using Cypress or Playwright.
Bumps the .nvmrc to use Node v18.18.0 for day to day activities.
BONUS Adds the ability to record video on failing Cypress tests and upload video to Buildkite artifacts.

PR closes #7022

QA

Docker container QA

QA will be verified manually on the Buildkite UI to ensure all tests pass with this new Docker image.

`.nvmrc` Node QA

This is a two-part process that shouldn't take more than 15-20 minutes.

Part I
Pull the branch locally, then enter the following commands on your terminal:

nvm install 18.18.0
yarn you may see a Node warning about fsevents optional dependency error. This is b/c of our older Jest dep.
yarn lint
yarn test-unit
yarn test-cypress

Part II
When all of the following tests complete successfully:

nvm alias default 18.18.0
nvm uninstall 18.17.1

1Copenut · 2023-10-10T20:10:35Z

Buildkite test this

.buildkite/scripts/pipeline_test.sh

1Copenut · 2023-10-10T22:08:52Z

package.json

@@ -32,7 +32,6 @@
    "test": "yarn lint && yarn test-unit",
    "test-ci": "yarn test && yarn test-cypress",
    "test-unit": "node ./scripts/test-unit",
-    "test-a11y": "node ./scripts/a11y-testing",


I removed the command to call our older Puppeteer tests to run a11y checks against the docs pages. I haven't removed the script because it'll be repurposed with Cypress at some point in the future.

I would strongly rather see us rip it all out now (the script file, the start-test-server-and-a11y-test script, and the @axe-core/puppeteer+puppeteer dependencies.

There's honestly very little chance we'll end up using Puppeteer in the future. We'll likely either use Cypress E2E testing directly to test our docs, or we'll use Playwright instead (since Playwright is more lightweight and actually supports M1 machines).

There's virtually no upside to keeping Puppeteer in our dependencies - it causes issues for devs on arm64 machines and it increases CI load times installing a dependency we're not even using.

Puppeteer, script, and deps should be removed now. I wasn't 💯 that the start-test-server command in the package.json file was specific to Puppeteer, so it stayed in. Happy to remove it if it seems unnecessary.

start-test-server is specific to puppeteer/the a11y script, please nuke it!

I'm also fairly sure we can rip out the webpack-dev-server dependency as well once that's gone; the script is the only thing using it. The wins just keep on coming 🎉 💥

Yes on start-test-server but we'll need to keep webpack-dev-server to run yarn start. Adding it back now as I bump the .nvmrc.

Lol fuck I totally missed that webpack serve is powered by webpack-dev-server, I just did a basic grep for the package name 🤦 Sorry!!

Hey, I missed it too. Adding it back in a jiffy.

scripts/docker-ci/Dockerfile

cee-chen · 2023-10-10T23:23:10Z

scripts/docker-ci/Dockerfile

-    && chown -R pptruser:pptruser /home/pptruser \
-    && chown -R pptruser:pptruser /node_modules \
-    && chown -R pptruser:pptruser /cypress


I'm not confident this change is 1:1 - in particular -G audio,video seems potentially odd that we've lost it. Do you mind explaining how/why this works, and also pushing up a Cypress test change to fail deliberately so we can ensure that screenshots/artifacts are still captured?

I pulled the commands apart this morning with some help from the Linux man pages. To the best of my understanding we're safe to remove the commands as are. the -G audio,video was being used to assign the pptruser to the audio and video groups to enable audio and video recordings.

Cypress does allow video recordings, but the feature is turned off by default. If we want to start video recording in the future, we'd need to add back the -G audio,video to the node user.

I've pushed a commit d606b47 that purposely fails a Cypress test. The ~~should output a screenshot~~ Docker image captures screenshots and they're uploaded by the Buildkite runner under the Artifacts tab.

Cypress does allow video recordings, but the feature is turned off by default. If we want to start video recording in the future, we'd need to add back the -G audio,video to the node user.

Let's do that pre-emptively please. I'd like the ability to quickly set Cypress in CI to emit videos for easier debugging if necessary via cypress.json without having to know linux/docker settings or getting confused as to why no videos are being output.

scripts/docker-ci/Dockerfile

This reverts commit d606b47.

wiki/contributing-to-eui/testing/automated-accessibility-testing.md

cee-chen · 2023-10-12T14:50:36Z

🎉 🎉 This is looking really fantastic - thank you Trevor! Really excited for this cleanup. Can we do one more thing to test the video behavior:

In a [REVERT ME] commit, deliberately make any test fail, then update cypress.config.ts to set video: true
Wait for buildkite tests to finish running and confirm that both a screenshot and video were uploaded and are viewable for debugging purposes
Revert the commit again

Once the above steps are complete, let's go ahead and merge away on this PR!

…v dependency.

1Copenut · 2023-10-12T21:53:42Z

Once the above steps are complete, let's go ahead and merge away on this PR!

I ran a quick test with a failing spec and verified the video is being created and added to Buildkite artifacts. That's the good news. The bad news is Cypress 12 doesn't allow me to only keep videos of failing tests, so we had a lot of MP4s in that last run.

Cypress 13 has an official solution for only keeping failing test runs, so I'll be sure to include that change with work to bump us to v13.3.

package.json

…de` instead - answer found in https://stackoverflow.com/a/76733478

- pretty basic/expected, there are other instances in EUI where we've had to do this

- one less dependency to keep up to date

cee-chen

🎉 Thanks for bearing with me on feedback rounds Trevor - this is an enormously wonderful cleanup!

kibanamachine · 2023-10-13T16:30:41Z

Preview staging links for this PR:

Docs site: https://eui.elastic.co/pr_7274/
Storybook: https://eui.elastic.co/pr_7274/storybook

elasticmachine · 2023-10-13T16:30:47Z

💚 Build Succeeded

Buildkite Build
Commit: 04e4d16

History

💚 Build #824 succeeded 72cd3cb
💚 Build #823 succeeded 01c7bce
💔 Build #822 failed 4edbee5
💔 Build #821 failed 66526b8
💚 Build #819 succeeded 6d01521
💚 Build #818 succeeded 1012b27

cc @1Copenut

[CHORE] Bumping Docker to use node-18.18.0-slim base image.

9a67186

1Copenut added skip-changelog infrastructure labels Oct 10, 2023

1Copenut self-assigned this Oct 10, 2023

1Copenut added tech debt chore labels Oct 10, 2023

1Copenut mentioned this pull request Oct 10, 2023

[CHORE] Bump .nvmrc to Node v18.18.0. #7276

Closed

7 tasks

1Copenut commented Oct 10, 2023

View reviewed changes

1Copenut marked this pull request as ready for review October 10, 2023 22:09

1Copenut requested a review from a team as a code owner October 10, 2023 22:09

cee-chen reviewed Oct 10, 2023

View reviewed changes

scripts/docker-ci/Dockerfile Outdated Show resolved Hide resolved

cee-chen reviewed Oct 10, 2023

View reviewed changes

scripts/docker-ci/Dockerfile Show resolved Hide resolved

1Copenut added 7 commits October 11, 2023 10:25

Added clarifying comments to Dockerfile.

908fbd9

Revert. Purposely failing spec to verify screenshot.

d606b47

Revert purposely failing spec to verify screenshot.

b89e5fe

This reverts commit d606b47.

Removing Puppeteer dependencies and script calls.

7514cd8

Deleting Puppeteer script for running a11y checks on docs pages.

4252d88

Updating automated a11y testing docs.

3820f9b

Bumping nvmrc to 18.18.0 for daily Node usage.

1012b27

1Copenut changed the title ~~[CHORE] Bumping Docker to use node-18.18.0-slim base image.~~ [CHORE] Bump Docker and .nvmrc to use Node v18.18.0 Oct 11, 2023

cee-chen reviewed Oct 12, 2023

View reviewed changes

wiki/contributing-to-eui/testing/automated-accessibility-testing.md Show resolved Hide resolved

1Copenut added 3 commits October 12, 2023 08:49

Removed last Puppeteer script call and dev dependency.

06b46b0

Bumping Node image. Added node user to audio, video groups.

5f8b874

Upgrading scripts to use Docker container v5.5.

6d01521

1Copenut changed the title ~~[CHORE] Bump Docker and .nvmrc to use Node v18.18.0~~ [CHORE] Bump Docker and .nvmrc to use Node v18.18.1 Oct 12, 2023

1Copenut added 2 commits October 12, 2023 10:31

Moving to Node v18.18.1 for daily use. Restored webpack-dev-server de…

9e04999

…v dependency.

Added pinned ws type to remove linting error.

509b13e

1Copenut added 5 commits October 12, 2023 13:13

Added option to record video on Cypress spec failure.

fd1dea9

[REVERT] Purposely failing Cypress spec to test video artifact.

b532bea

Forgot to set video record to true.

66526b8

Removing after:spec logic not necessary until Cypress 13 migration.

4edbee5

Reverting purposely failing test and video output after confirmation.

01c7bce

Final dependency version cleanup.

72cd3cb

cee-chen reviewed Oct 13, 2023

View reviewed changes

package.json Outdated Show resolved Hide resolved

1Copenut and others added 4 commits October 13, 2023 10:42

Merge branch 'main' into chore/docker-node-18.18.0

1bcd40f

Remove unnecessary @types/ws dependency/pin by upgrading `@types/no…

e24cb34

…de` instead - answer found in https://stackoverflow.com/a/76733478

Fix type error from upgrading @types/node

9d5cb93

- pretty basic/expected, there are other instances in EUI where we've had to do this

Remove @types/node entirely and let npm figure out the types it needs

04e4d16

- one less dependency to keep up to date

cee-chen changed the title ~~[CHORE] Bump Docker and .nvmrc to use Node v18.18.1~~ Bump Node to v18.18.1; Remove puppeteer Oct 13, 2023

cee-chen approved these changes Oct 13, 2023

View reviewed changes

1Copenut merged commit 2de79ad into elastic:main Oct 13, 2023
7 checks passed

1Copenut deleted the chore/docker-node-18.18.0 branch October 13, 2023 16:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump Node to `v18.18.1`; Remove puppeteer #7274

Bump Node to `v18.18.1`; Remove puppeteer #7274

1Copenut commented Oct 10, 2023 •

edited by cee-chen

Loading

1Copenut commented Oct 10, 2023

1Copenut Oct 10, 2023

cee-chen Oct 10, 2023 •

edited

Loading

1Copenut Oct 11, 2023

cee-chen Oct 12, 2023 •

edited

Loading

cee-chen Oct 12, 2023 •

edited

Loading

1Copenut Oct 12, 2023

cee-chen Oct 12, 2023

1Copenut Oct 12, 2023

cee-chen Oct 10, 2023

1Copenut Oct 11, 2023 •

edited

Loading

cee-chen Oct 12, 2023

1Copenut Oct 12, 2023

cee-chen commented Oct 12, 2023 •

edited

Loading

1Copenut commented Oct 12, 2023

cee-chen left a comment

kibanamachine commented Oct 13, 2023

elasticmachine commented Oct 13, 2023

Bump Node to v18.18.1; Remove puppeteer #7274

Bump Node to v18.18.1; Remove puppeteer #7274

Conversation

1Copenut commented Oct 10, 2023 • edited by cee-chen Loading

Summary

QA

Docker container QA

.nvmrc Node QA

1Copenut commented Oct 10, 2023

Choose a reason for hiding this comment

cee-chen Oct 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cee-chen Oct 12, 2023 • edited Loading

Choose a reason for hiding this comment

cee-chen Oct 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1Copenut Oct 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cee-chen commented Oct 12, 2023 • edited Loading

1Copenut commented Oct 12, 2023

cee-chen left a comment

Choose a reason for hiding this comment

kibanamachine commented Oct 13, 2023

elasticmachine commented Oct 13, 2023

💚 Build Succeeded

History

Bump Node to `v18.18.1`; Remove puppeteer #7274

Bump Node to `v18.18.1`; Remove puppeteer #7274

1Copenut commented Oct 10, 2023 •

edited by cee-chen

Loading

`.nvmrc` Node QA

cee-chen Oct 10, 2023 •

edited

Loading

cee-chen Oct 12, 2023 •

edited

Loading

cee-chen Oct 12, 2023 •

edited

Loading

1Copenut Oct 11, 2023 •

edited

Loading

cee-chen commented Oct 12, 2023 •

edited

Loading