-
Notifications
You must be signed in to change notification settings - Fork 0
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
Dependency Updates and Node 20 Support (Node 14 deprecation) #68
Conversation
this loads the custom docker image, built with buildx, into local system container namespace. necessary when updating dockerfile without the option to publish to docker registry.
`genConfig` is now `makeConfig` and is ultimately used in `backstop init`. There is no need to test this separately, as it's part of the overall flow of `init`, which is now used in npm's `integration-test` script, and passing.
This was originally part of the `gulp` workflow, back in the `slimer` and `casper` days. https://github.com/garris/BackstopJS/blob/b00ab5bf358fcb3b77724ac4ef10b30e86b5188d/core/command/echo.js
…on, smoke, and sanity tests This provides a final outcome message for all npm scripts for internal project testing. With tests that `exit 1`, a caution message is used, as opposed to a full-blown "FAIL".
also renames "docker-load" script "load-docker" to align with other script naming conventions
Wow @dgrebb this is great! I am traveling today and tomorrow. Will give this a look later in the week and set up a time to sync on next steps. Cheers! |
@dgrebb thanks for your patience -- this PR is really beautiful. I love all the GH test flows and thoughtfulness you put into the readme file also. There is so much going on in this PR -- and a lot of downstream dependencies. Would it be possible to book some time to do a merge session? I think we might need an hour. Next Friday would be easiest for me (otherwise my head might explode). Would that work? |
Sounds good @garris — no rush! Big updates deserve time and care. Thank you for the kind words :) I'll get back to you with some times. Are you back in PT? End-of day your time better? Also, if it makes things easier, we could split it up: Quick Merge: General Fixes and Enhancements (no dependency changes)
Quick Merge: Test Enhancements
Code Review: Dependency/Node Update (dependency changes)
|
Yes, chunking it up like this makes sense. I can easily do 2p & later (WC time) if that works for you. Thanks! |
@garris happy Monday!
Here is the (drafted) PR for Node 20 support, and Node 14 deprecation. Test results and CI/CD can be seen here.
Please let me know if you'd like to discuss this week, would like any changes, and have any feedback in general :)
Passing Tests
I set up a collection of GitHub Actions workflows, to make testing awareness and communication more streamlined for the project. More on that in a soon-to be separate PR :)
The above results are from a testing-only branch, which is rebased from my
develop
to include the GitHub testing workflows. This branch doesn't have actions merged in, so it can't be tested without polluting the PR.Happy to explain more, but perhaps an image will suffice!
Backstop Tests
Docker Tests
The Node 20 Docker image is available here, and can be tested locally via:
This assumes
--platform linux/arm64
. Please update to test on non-arm architecture!Also, one may need to change the
--mount
source
value back tocwd
— I wasn't sure why that is used in the README example for running against a custom image.Note
The above docker image and test results are in a
develop
rebased branch for testing only.The
Dockerfile
in this branch is set to usedgrebb/BackstopJS#feature/34-node-20
as the installation source forbackstopjs
. This was needed to demonstrate passing tests with the latest changes, inside GitHub Actions (coming soon in another PR).This closes #34.
Details
Change Summary
This comes with many dependency updates, formatting adjustments, a README refactor,
Dockerfile
updates, and general bugfixes, removals, and enhancements. Here's the tl;dr:eslint-disable
where neededengines
inpackage.json
Dockerfile
npx playwright install --with-deps
inDockerfile
npm
script:integration-test
load-docker
npm script for local testing of built docker imagesload-docker
genConfig
npm scriptmakeConfig
, akabackstop init
)I can squash the 14 commits down into one with a longer description attached to the squash-commit. Let me know and it shall be done!
Next Steps
README.md Updates
While updating the README, I thought it could use a refresh. Happy to revert this if needed.
I'm using a handy VSCode Markdwon plugin for TOC generation, which automatically creates and updates based on comments and
.vscode/settings.json
configuration. Happy to share details if curious!HTML Report Development
Added a warning here that currently builds are broken and updates are in progress to fix webpack, update React, etc.
Docker
Added a note about
load-docker
npm script to build/run Dockerfile changes locally without publishing.General Spacing/Indentation/Enhancements
Removed some legacy, commented-out sections from the README, which I think are OK to keep in git history for now?
Removed
$
in command examples that users will likely want to copy/paste.Converted scenario properties section to a markdown table.
Converted
onBeforeScript
/onReadyScript
variable list to a markdown table.Added some fancy GitHub-flavored markdown
blockquote
highlights. Great for "warnings", "tips", and other callouts.Notes
Critical Vulnerability in
mockery
The maintainer of
mockery
has not addressed this yet, but a patch is open. Please let me know if you're interested in addingpatch-package
and apatches
directory to address this. Happy to set that up!Questions
docker buildx
executions with different tags innpm run build-docker
— is this needed for the Docker registry?