-
Notifications
You must be signed in to change notification settings - Fork 68
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
Resume sending E2E test failures to slack #3575
Conversation
* Move user management functions to env setup * Update user management script order + moved values to vars
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.
LGTM, left a comment for my understanding.
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.
LGTM.
name: screenshots | ||
path: screenshots | ||
name: e2e-screenshots | ||
path: tests/e2e/screenshots |
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.
@dechov We moved to use the useE2EJestConfig
provided by e2e-environment
which sends the failed screenshot messages directly to #wcpay-bots
channel in slack, which might have overridden the old artifact behavior to only create artifacts for failure cases, see this run for reference.
Do we need the screenshots for success scenarios as well?
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.
Ah, I see 👍
Not that we need the regular ones but there is some theoretical benefit to having snapshots from each distinct screen from the normal run, to ensure that there aren't visual regressions that wouldn't be caught by specific assertions. What do you think of having a "failure screenshots" artifact in addition to the earlier one?
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 feel unless there is an alert/checking process in place, we wouldn't really monitor the successful ones regularly, and manually checking visual regressions could be tricky across runs.
We could either ask GS to do some sanity or have explicit visual regression tests in place for the same but that's just my opinion, we could discuss it internally to see what other folks feel about the same.
Fixes #3555
Changes proposed in this Pull Request
e2e-environment
to resume sending E2E test failures to slack.jest-setup.js
tojest.setup.js
and move it totests/e2e/config
for it to be picked up correctly.useE2EJestConfig
for jest config to use the failure script that includes slack reporting.Testing instructions
update/slack-e2e-reporting
.#wcpay-bots
channel in slack, reference job.Sample slack screenshot
readme.txt
andchangelog.txt
(or does not apply)Post merge