-
Notifications
You must be signed in to change notification settings - Fork 145
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
👷 fix tags to match the test optimisation configuration attributes #3337
base: main
Are you sure you want to change the base?
Conversation
Bundles Sizes Evolution
🚀 CPU Performance
🧠 Memory Performance
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3337 +/- ##
=======================================
Coverage 92.75% 92.75%
=======================================
Files 299 299
Lines 7852 7852
Branches 1792 1792
=======================================
Hits 7283 7283
Misses 569 569 ☔ View full report in Codecov by Sentry. |
a401af3
to
26d2b9e
Compare
783f5f1
to
227df2f
Compare
test.beforeEach(({ browserName }, testInfo) => { | ||
testInfo.skip(browserName !== 'chromium', 'only chromium supports touch gestures emulation for now (via CDP)') | ||
}) |
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.
skipping in beforeEach
prevent createTest()
to run, which is nice but then the tags used for CI optimisation fingerprinting are not set.
@@ -195,22 +195,6 @@ describe('vitalCollection', () => { | |||
expect(rawRumEvents[0].customerContext).toEqual({ foo: 'bar' }) | |||
}) | |||
|
|||
it('should create a vital from add API', () => { |
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.
Duplicate test, see line 255
@@ -263,19 +247,6 @@ describe('vitalCollection', () => { | |||
expect((rawRumEvents[0].rawRumEvent as RawRumVitalEvent).vital.description).toBe('baz') | |||
expect(rawRumEvents[0].customerContext).toEqual({ foo: 'bar' }) | |||
}) | |||
|
|||
it('should discard a vital for which a frozen state happened', () => { |
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.
Duplicate test, see line 214
227df2f
to
0b99993
Compare
89e7bd3
to
c117f1c
Compare
Motivation
The main objective of this PR is to play nice with CI Optimization product. It uses a combination of configuration attribute and test name to identify unique test and detects flakynes by checking if a unique test run more than once with different status for a give commit (So if we only discriminate test by name, a test that fails on Firefox but passes on Chrome would be incorrectly detected as flaky).
With the change on this PR, for a given commit, we should see only one test run per fingerprint (unless a job was retried). Playwright has build-in duplicate test name detection, but not Karma/Jasmine, so I created a simple reporter plugin to log an error if a test is duplicated. Reporters can't fail a job, so it's only a log, but it can help detecting them faster.
Another issue, is that playwright does not report all test run when it retries a test, so CI Optimization has no visibility on flaky test (unless the job is retries). To mitigate the issue I'm adding a
test.flaky
tag at tearDown when we detect a job has be retried. It won't appear in CI Optimisation but we can at least make a dashboard for itChanges
Testing
I have gone over the contributing documentation.