Skip to content

Conversation

@Lms24
Copy link
Member

@Lms24 Lms24 commented Apr 17, 2024

Our flaky test detector uses a couple of heuristics to determine how many times each changed test should be run. We previously relied on the assumption that one test path (file) only contained one file. In some cases this is not true because it's totally valid to contain multiple sentryTest test runs in one file.

This PR adds a small test detection heuristic to the detector to roughly count the number of tests in a file. This should reduce the number of times each test is running in case files with multiple tests have changed. Furthermore, this PR also refactors the run count determination logic a bit to make it easier to understand.

image

An example of this is a PR of mine (#11631) where the test detector would run into the 60 minutes timeout because each changed test file contained multiple (~5) test files. So the actual number of tests would be 5x higher than what our heuristic assumed.

/**
* The number of browsers we run the tests in.
*/
const NUM_BROWSERS = 3;
Copy link
Member Author

Choose a reason for hiding this comment

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

So, we previously set this to 4 but afaict at max, we test 3 browsers. Some tests potentially even only run on Chrome. So I think it's ok to reduce this in favour of more often running each test. My super unscientific testing says we currently run way shorter than 30min.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 17, 2024

size-limit report 📦

Path Size
@sentry/browser 21.67 KB (0%)
@sentry/browser (incl. Tracing) 31.46 KB (0%)
@sentry/browser (incl. Tracing, Replay) 66.79 KB (0%)
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 60.19 KB (0%)
@sentry/browser (incl. Tracing, Replay with Canvas) 70.62 KB (0%)
@sentry/browser (incl. Tracing, Replay, Feedback) 80.5 KB (0%)
@sentry/browser (incl. Feedback) 35.25 KB (0%)
@sentry/browser (incl. Feedback, Feedback Modal) 35.25 KB (0%)
@sentry/browser (incl. Feedback, Feedback Modal, Feedback Screenshot) 37.28 KB (0%)
@sentry/browser (incl. sendFeedback) 26.46 KB (0%)
@sentry/react 24.35 KB (0%)
@sentry/react (incl. Tracing) 34.37 KB (0%)
@sentry/vue 25.2 KB (0%)
@sentry/vue (incl. Tracing) 33.18 KB (0%)
@sentry/svelte 21.79 KB (0%)
CDN Bundle 24.03 KB (0%)
CDN Bundle (incl. Tracing) 32.76 KB (0%)
CDN Bundle (incl. Tracing, Replay) 66.4 KB (0%)
CDN Bundle (incl. Tracing, Replay, Feedback) 82.6 KB (0%)
CDN Bundle - uncompressed 70.86 KB (0%)
CDN Bundle (incl. Tracing) - uncompressed 97.59 KB (0%)
CDN Bundle (incl. Tracing, Replay) - uncompressed 207.25 KB (0%)
@sentry/nextjs (client) 33.7 KB (0%)
@sentry/sveltekit (client) 31.96 KB (0%)
@sentry/node 155.86 KB (0%)

@Lms24 Lms24 marked this pull request as ready for review April 17, 2024 13:00
@Lms24 Lms24 requested review from lforst and mydea April 17, 2024 13:00
@Lms24 Lms24 merged commit 308e743 into develop Apr 17, 2024
@Lms24 Lms24 deleted the lms/ci-adjust-detectFlakyTests branch April 17, 2024 15:51
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.

3 participants