-
Notifications
You must be signed in to change notification settings - Fork 153
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
Deploy playwright reports #4261
Conversation
Storybook staging is available at https://kiwicom-orbit-dsil-deploy-playwright-reports.surge.sh |
Size Change: +4 B (0%) Total Size: 458 kB
ℹ️ View Unchanged
|
Deploying with Cloudflare Pages
|
05bd782
to
38f0b33
Compare
7523f49
to
1582563
Compare
.github/workflows/test.yml
Outdated
test-ct-report: | ||
name: Visual Regression Tests Report | ||
runs-on: ubuntu-latest | ||
if: always() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if there's a cleaner way to run this job only when test-ct
fails instead of always and do the checks inside.
Have you considered something like this? (untested)
if: ${{ needs.test-ct.result == 'failure' }}
Disregard this comment if you find it so cumbersome, we can tackle this in future iterations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the official docs, they recommend using always()
. I have tried several other approaches and they all failed in some case. I did not try that one. I can give it a quick try
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it works for when all tests pass. Will now test with failing tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed IRL, if: ${{ always() && needs.test-ct.result == 'failure' }}
does the trick. So we will go with that and the simplification of the run
part, removing the verification for the artifacts existence
1582563
to
5f9d504
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's more than enough for a first approach, approving 👍
Feel free to postpone other improvements for the future.
03aa1f9
to
8d02386
Compare
This will remove the test report once the PR is closed
8d02386
to
607f5cd
Compare
Playwright reports are deployed with surge when visual tests fail.
On PR close, they are deleted.
I forced one Tag test to fail (on a now deleted commit), and it's possible to see the pipeline execution here. The tests fail and the report is deployed here.
If no tests fail, no report is deployed. Example here.
The deletion pipeline will be executed after this PR is merged (and will delete the report this generated).
FEPLT-1942