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

Add npm script for updating playwright snapshots #279

Merged
merged 1 commit into from
Aug 10, 2023

Conversation

kennethnym
Copy link
Member

Description

This PR adds an NPM script that can update playwright snapshots.

  • playwright:update-snapshot is the new script
  • test:playwright is now playwright:test
  • Dockerfile is updated in order to allow additional args to be passed to yarn playwright test
  • A new env var LOCAL_DOCKER is created to denote whether the playwright docker container is run locally.
    • The playwright config utilizes LOCAL_DOCKER to determine which host to serve the report on.

Testing instructions

Add a set up instructions describing how the reviewer should test the code

  • Review code
  • Check Actions build
  • Review changes to test coverage

Agile board tracking

Hot fix.

@codecov
Copy link

codecov bot commented Aug 4, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (6bf2ff6) 95.77% compared to head (502de35) 95.77%.
Report is 9 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #279   +/-   ##
========================================
  Coverage    95.77%   95.77%           
========================================
  Files           75       75           
  Lines         3170     3170           
  Branches       897      897           
========================================
  Hits          3036     3036           
  Misses         132      132           
  Partials         2        2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@louise-davies louise-davies left a comment

Choose a reason for hiding this comment

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

LGTM - I just had a question on how the update snapshot command works

"serve:build": "yarn build && serve -l 5001 build",
"test": "craco test --env=jsdom --coverage --watchAll=false",
"test:watch": "craco test --env=jsdom --watch",
"test:playwright": "docker build --tag operations-gateway-playwright-test --file Dockerfile.playwright . && docker run --volume ./playwright-report:/app/playwright-report --publish 9323:9323 operations-gateway-playwright-test",
"playwright:test": "yarn build:playwright-docker && docker run --env LOCAL_DOCKER=true --volume ./playwright-report:/app/playwright-report --publish 9323:9323 operations-gateway-playwright-test",
"playwright:update-snapshot": "yarn build:playwright-docker && docker run --env LOCAL_DOCKER=true --volume .:/app --publish 9323:9323 operations-gateway-playwright-test",
Copy link
Member

Choose a reason for hiding this comment

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

I tested that this does actually update snapshots - but how does it? I can't see any flags being passed to the playwright command...

Copy link
Member Author

Choose a reason for hiding this comment

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

I passed in a different --volume flag for playwright:test and playwright:update-snapshot. Since updated snapshots are written inside the e2e folder, in order to write the changes to e2e/ and playwright-report/ folders to the host file system, I need to mount the entire project folder, hence --volume .:/app.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, but doesn't playwright only update snapshots if you provide the -u flag?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question, how did both of us manage to get the snapshots updating without the flag?? I will make another PR to add it back in.

@kennethnym kennethnym merged commit aed5a19 into develop Aug 10, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants