Skip to content

Conversation

redsun82
Copy link
Contributor

No description provided.

@github-actions github-actions bot added the Rust Pull requests that update Rust code label Nov 29, 2024
@redsun82 redsun82 requested a review from a team as a code owner November 29, 2024 16:16
@redsun82 redsun82 force-pushed the redsun82/rust-windows-flaky-test branch from 0533415 to 4889032 Compare November 29, 2024 16:23
@redsun82 redsun82 changed the title Rust: test running windows flaky test multiple times Rust: fix windows flakiness Nov 29, 2024
Copy link
Contributor

@paldepind paldepind left a comment

Choose a reason for hiding this comment

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

Looks great. I've left a suggestion on the comment which I think makes it a bit easier to understand :)

Comment on lines +201 to +202
// There seems to be some flaky inconsistencies around UNC paths on Windows, so if we fail to
// find the file id for a UNC path like that, we try to canonicalize it using dunce then.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// There seems to be some flaky inconsistencies around UNC paths on Windows, so if we fail to
// find the file id for a UNC path like that, we try to canonicalize it using dunce then.
// There seems to be some flaky inconsistencies around UNC paths on Windows, so if we fail to
// find the file id for a UNC path, we try with a canonicalized (by dunce) path as well.

Copy link
Contributor Author

@redsun82 redsun82 Nov 29, 2024

Choose a reason for hiding this comment

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

Makes sense! I'll do a follow up though on Monday, I prefer to not have the flakiness in for the nightlies 🙂

@redsun82 redsun82 merged commit 7cd344c into main Nov 29, 2024
26 checks passed
@redsun82 redsun82 deleted the redsun82/rust-windows-flaky-test branch November 29, 2024 21:04
redsun82 pushed a commit that referenced this pull request Dec 2, 2024
This is a follow up to #18167, addressing a
review comment from @paldepind.
redsun82 pushed a commit that referenced this pull request Dec 2, 2024
This is a follow up to #18167, addressing a
review comment from @paldepind.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants