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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: infer tmp directory name from test name everywhere #6566

Merged
merged 5 commits into from May 13, 2024
Merged

Conversation

Skn0tt
Copy link
Member

@Skn0tt Skn0tt commented May 8, 2024

We have some flaky tests on main, that are being flaky because they're reusing the same directory across multiple, concurrent tests. In bad race conditions, this means the directory is being cleaned up by one test, while a different test still uses it 馃挜

fa5de3b contains a fix, where we're not specifying the directory name by hand, but we use the test name as a basis for it. And because i'm a little annoyed this happened in the first place, i'm also replacing all other usage of withSiteBuilder(string) in f625203.

@Skn0tt Skn0tt self-assigned this May 8, 2024
@Skn0tt Skn0tt requested a review from a team as a code owner May 8, 2024 10:48
Copy link

github-actions bot commented May 8, 2024

馃搳 Benchmark results

Comparing with 5e716c0

  • Dependency count: 1,347 (no change)
  • Package size: 312 MB (no change)
  • Number of ts-expect-error directives: 993 (no change)

ericapisani
ericapisani previously approved these changes May 8, 2024
@ericapisani
Copy link
Contributor

There's a test failure that looks like it might be legit but otherwise LGTM

mrstork
mrstork previously approved these changes May 8, 2024
@Skn0tt Skn0tt dismissed stale reviews from mrstork and ericapisani via 2f41cdd May 13, 2024 13:04
Copy link
Contributor

@mrstork mrstork left a comment

Choose a reason for hiding this comment

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

Awesome! We might just change the title before merging is all

@Skn0tt Skn0tt changed the title fix: unflake concurrent tests with race conditions on the test directory chore: infer tmp directory name from test name everywhere May 13, 2024
@Skn0tt Skn0tt enabled auto-merge (squash) May 13, 2024 13:21
@Skn0tt Skn0tt merged commit a9bd5e2 into main May 13, 2024
48 checks passed
@Skn0tt Skn0tt deleted the fix-flaky-test branch May 13, 2024 13:30
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.

None yet

3 participants