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

test(playwright): enable github reporter, test retries #27961

Merged
merged 12 commits into from
Aug 10, 2023

Conversation

liamdebeasi
Copy link
Contributor

@liamdebeasi liamdebeasi commented Aug 9, 2023

Issue number: N/A


What is the current behavior?

The team would like to explore solutions for being informed of flaky tests in a way that is not disruptive to our workflow. Currently, flaky tests fail immediately which means we have to re-run them every time. We'd like flaky tests to be automatically retried but also reported to us so we can address them in a separate PR.

What is the new behavior?

  • Enables the Playwright GitHub reporter. This will report about flaky tests on the PR if applicable as well as in the CI results.
  • Enables test retries. Tests will be retried up to 2 times before failing.
  • Disables reporting slow tests in the GitHub reporter. Some of our tests require gesture interaction which are inherently slow but otherwise working as intended. We don't necessarily need to know about these right now.
  • Disables "maxFailures". Tests that can fail at most once are never detected as flaky since they are never retried. As a result, we need to disable this in order to have flaky tests be reported to us.

Does this introduce a breaking change?

  • Yes
  • No

Other information

@stackblitz
Copy link

stackblitz bot commented Aug 9, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@github-actions github-actions bot added the package: core @ionic/core package label Aug 9, 2023
@liamdebeasi liamdebeasi changed the title test github reporter test(playwright): enable github reporter, test retries Aug 10, 2023
@liamdebeasi liamdebeasi marked this pull request as ready for review August 10, 2023 17:05
@liamdebeasi liamdebeasi requested review from a team and mapsandapps and removed request for a team August 10, 2023 17:05
@liamdebeasi liamdebeasi added this pull request to the merge queue Aug 10, 2023
@@ -63,13 +63,16 @@ const config: PlaywrightTestConfig = {
/* Fail the build on CI if you accidentally left test.only in the source code. */
forbidOnly: !!process.env.CI,
/* Fail fast on CI */
Copy link
Contributor

Choose a reason for hiding this comment

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

i think this comment needs to be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! Addressed in #27973

Merged via the queue into main with commit b6f43e0 Aug 10, 2023
@liamdebeasi liamdebeasi deleted the playwright-reporter branch August 10, 2023 17:38
github-merge-queue bot pushed a commit that referenced this pull request Aug 10, 2023
Issue number: #

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->

Shawn noted that I had an outdated comment in the `playwright.config.ts`
file when working on
#27961:
#27961 (review)
 
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Remove the old comment

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core @ionic/core package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants