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

ci: flake detection should run in both bundlers #72773

Merged
merged 3 commits into from
Nov 13, 2024

Conversation

ztanner
Copy link
Member

@ztanner ztanner commented Nov 13, 2024

We run flake detection on PRs to help catch flaky tests before they hit canary, but we currently only run flaky test detection in Webpack and not Turbopack. This is problematic because there's sufficient forking logic in tests for the different bundlers & we've seen a high likelihood of a test to flake more frequently in one bundler vs another due to things like differing compilation speeds and differing implementation of core features.

This will add some additional time to this CI job to account for needing to run the tests 6 times, but will hopefully catch things before they become a problem later. I think we could also consider reducing the number of times per bundler to be 2 if it turns out to be adding too much time.

This updates the flake detection job to run the tests multiple times in Turbopack as well.

CI run: https://github.com/vercel/next.js/actions/runs/11822662639/job/32940264160?pr=72773

@ijjk ijjk added created-by: Next.js team PRs by the Next.js team. tests labels Nov 13, 2024
Copy link
Member Author

ztanner commented Nov 13, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @ztanner and the rest of your teammates on Graphite Graphite

@ztanner ztanner marked this pull request as ready for review November 13, 2024 17:39
@ztanner ztanner requested review from ijjk and gaojude November 13, 2024 17:47
Copy link
Member

@ijjk ijjk left a comment

Choose a reason for hiding this comment

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

Do we need to parallelize more to avoid these being slow?

@ztanner
Copy link
Member Author

ztanner commented Nov 13, 2024

Do we need to parallelize more to avoid these being slow?

I think we'll need to see how it goes -- I picked a beefier test to modify and it finished in dev/start flake detection before prod tests finished. I think the current parallelization often goes under utilized because we typically only see a few test changes per PR, but if this causes problems I'd probably want to split the Turbopack flake detection into a separate job

@ztanner ztanner enabled auto-merge (squash) November 13, 2024 17:51
@ijjk
Copy link
Member

ijjk commented Nov 13, 2024

Failing test suites

Commit: 108d027

__NEXT_EXPERIMENTAL_PPR=true pnpm test-dev test/development/app-hmr/hmr.test.ts (PPR)

  • app-dir-hmr > filesystem changes > should update server components after navigating to a page with a different runtime
Expand output

● app-dir-hmr › filesystem changes › should update server components after navigating to a page with a different runtime

expect(received).toBe(expected) // Object.is equality

Expected: "ipad"
Received: "mac"

  65 |       await next.patchFile(envFile, 'MY_DEVICE="ipad"', async () => {
  66 |         await retry(async () => {
> 67 |           expect(await browser.elementByCss('p').text()).toBe('ipad')
     |                                                          ^
  68 |         })
  69 |
  70 |         const logs = await browser.log()

  at toBe (development/app-hmr/hmr.test.ts:67:58)
  at retry (lib/next-test-utils.ts:806:14)
  at development/app-hmr/hmr.test.ts:66:9
  at NextDevInstance.patchFile (lib/next-modes/base.ts:522:9)
  at NextDevInstance.patchFile (lib/next-modes/next-dev.ts:195:16)
  at Object.<anonymous> (development/app-hmr/hmr.test.ts:65:7)

Read more about building and testing Next.js in contributing.md.

@ztanner ztanner merged commit 6f10926 into canary Nov 13, 2024
104 of 110 checks passed
@ztanner ztanner deleted the 11-13-ci_flake_detection_should_run_in_both_bundlers branch November 13, 2024 18:15
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants