Skip to content

Drain file watcher events during test setup #24030

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.

Sign up for GitHub

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

Merged
zsol merged 1 commit into from
Mar 18, 2026
+18 −4
Merged

Drain file watcher events during test setup #24030

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions crates/ty/tests/file_watching.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,10 +456,24 @@ where
root_dir: root_path,
};

// Sometimes the file watcher reports changes for events that happened before the watcher was started.
// Do a best effort at dropping these events.
let _ =
test_case.try_take_watch_changes(|_event: &ChangeEvent| true, Duration::from_millis(100));
// Write a sentinel file to confirm the watcher is live and delivering events.
// This
// 1. ensures the watcher is working well, and not e.g. backed up with events unrelated to the current test
// 2. flushes events that are unrelated to the current test
let sentinel_path = project_path.join(".watcher_ready");
std::fs::write(sentinel_path.as_std_path(), "ready")?;
Copy link
Copy Markdown
Member

@MichaReiser MichaReiser Mar 18, 2026

Choose a reason for hiding this comment

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

Can you say more why this should fix the issue? I'm not very convinced that it does :)

ruff/crates/ty/tests/file_watching.rs

Lines 1532 to 1533 in 5879601

let changes = case.stop_watch(event_for_file("baz.py"));

does wait for a file system event related to baz.py, which we did observe in https://github.com/astral-sh/ruff/actions/runs/20860850768/job/59939819903?pr=22481, but the test still failed.

If the concern really is that it sometimes takes too long for the file watcher to start, then the solution is to bump the default timeout here

ruff/crates/ty/tests/file_watching.rs

Line 70 in 5879601

self.try_stop_watch(matcher, Duration::from_secs(10))

Copy link
Copy Markdown
Member Author

@zsol zsol Mar 18, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I think there are two things going for the approach in this PR:

  1. it ensures that the watcher is working well, and not e.g. backed up with events from before the current test
  2. flushes events unrelated to the current test a lot better than the previous test_case.try_take_watch_changes(|_| true, ...)

I suspect what's going on in https://github.com/astral-sh/ruff/actions/runs/20860850768/job/59939819903?pr=22481 is that there are multiple events emitted by the file watcher that aren't related to the symlink_inside_project test case, but still leak into them.

Edit: open to suggestions about how to better explain this in the comment :)

Copy link
Copy Markdown
Member

@MichaReiser MichaReiser Mar 18, 2026

Choose a reason for hiding this comment

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

Thank you. I think we can copy exactly what you wrote here into the comment.


test_case
.try_take_watch_changes(event_for_file(".watcher_ready"), Duration::from_secs(30))
.expect(
"Watcher failed to deliver sentinel event within 30s \
— file watching may not be operational",
);

// Clean up the sentinel file and drain its deletion event.
let _ = std::fs::remove_file(sentinel_path.as_std_path());
let _ = test_case
.try_take_watch_changes(event_for_file(".watcher_ready"), Duration::from_millis(500));

Ok(test_case)
}
Expand Down
Loading