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

Flaking test red_knot::file_watching directory_renamed #14473

Open
zanieb opened this issue Nov 20, 2024 · 4 comments
Open

Flaking test red_knot::file_watching directory_renamed #14473

zanieb opened this issue Nov 20, 2024 · 4 comments
Assignees
Labels
red-knot Multi-file analysis & type inference testing Related to testing Ruff itself

Comments

@zanieb
Copy link
Member

zanieb commented Nov 20, 2024

--- STDOUT:              red_knot::file_watching directory_renamed ---

running 1 test
test directory_renamed ... FAILED

failures:

failures:
    directory_renamed

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 23 filtered out; finished in 0.23s


--- STDERR:              red_knot::file_watching directory_renamed ---
thread 'directory_renamed' panicked at crates/red_knot/tests/file_watching.rs:646:5:
assertion failed: resolve_module(case.db().upcast(),
        &ModuleName::new_static("sub.a").unwrap()).is_none()
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

e.g. https://github.com/astral-sh/ruff/actions/runs/11925007919/attempts/1

@zanieb zanieb added the ci Related to internal CI tooling label Nov 20, 2024
@MichaReiser MichaReiser added testing Related to testing Ruff itself and removed ci Related to internal CI tooling labels Nov 20, 2024
@MichaReiser
Copy link
Member

Thanks. I think that was now the first time that it flaked and maybe it's related to the depot runner change. I don't plan on investing time unless it flakes more often (because it's very time intensive and mostly impossible to reproduce locally)

@AlexWaygood AlexWaygood added the red-knot Multi-file analysis & type inference label Nov 20, 2024
@zanieb
Copy link
Member Author

zanieb commented Nov 21, 2024

Sort of obvious, but you can reproduce the same error by removing the rename call

--- STDERR:              red_knot::file_watching directory_renamed ---
thread 'directory_renamed' panicked at crates/red_knot/tests/file_watching.rs:646:5:
assertion failed: resolve_module(case.db().upcast(),
        &ModuleName::new_static("sub.a").unwrap()).is_none()
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

   Canceling due to test failure
------------
     Summary [   0.181s] 1 test run: 0 passed, 1 failed, 3609 skipped
        FAIL [   0.181s] red_knot::file_watching directory_renamed
error: test run failed
❯ gd
diff --git a/crates/red_knot/tests/file_watching.rs b/crates/red_knot/tests/file_watching.rs
index 5c2880e0b..236d64b8d 100644
--- a/crates/red_knot/tests/file_watching.rs
+++ b/crates/red_knot/tests/file_watching.rs
@@ -635,8 +635,8 @@ fn directory_renamed() -> anyhow::Result<()> {
     let foo_baz = case.workspace_path("foo/baz");
 
     std::fs::create_dir(case.workspace_path("foo").as_std_path())?;
-    std::fs::rename(sub_path.as_std_path(), foo_baz.as_std_path())
-        .with_context(|| "Failed to move the sub directory")?;
+    // std::fs::rename(sub_path.as_std_path(), foo_baz.as_std_path())
+    //     .with_context(|| "Failed to move the sub directory")?;
 
     let changes = case.stop_watch();

or by removing the stop_watch call

--- STDERR:              red_knot::file_watching directory_renamed ---
thread 'directory_renamed' panicked at crates/red_knot/tests/file_watching.rs:646:5:
assertion failed: resolve_module(case.db().upcast(),
        &ModuleName::new_static("sub.a").unwrap()).is_none()
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

   Canceling due to test failure
------------
     Summary [   0.162s] 1 test run: 0 passed, 1 failed, 3609 skipped
        FAIL [   0.162s] red_knot::file_watching directory_renamed
error: test run failed
❯ gd
diff --git a/crates/red_knot/tests/file_watching.rs b/crates/red_knot/tests/file_watching.rs
index 5c2880e0b..12eb698c0 100644
--- a/crates/red_knot/tests/file_watching.rs
+++ b/crates/red_knot/tests/file_watching.rs
@@ -638,9 +638,9 @@ fn directory_renamed() -> anyhow::Result<()> {
     std::fs::rename(sub_path.as_std_path(), foo_baz.as_std_path())
         .with_context(|| "Failed to move the sub directory")?;
 
-    let changes = case.stop_watch();
+    // let changes = case.stop_watch();
 
-    case.apply_changes(changes);
+    // case.apply_changes(changes);
 
     // `import sub.a` should no longer resolve
     assert!(resolve_module(

So one of these two things could be failing (or something in the assertion like resolve_module). It seems dubious that the rename is not working as intended, so it's probably one of the other two? I'm not familiar with the process, so I can't guess why but it sounds like getting more information on failure in CI is a good next step.

I'd highly recommend setting up test-log and setting RUST_LOG so you can get some logs. I did this locally to see if there's more context and there's quite a bit of output.

@MichaReiser
Copy link
Member

I enabled extra logging in #14533. Please let me know if you see the flake in a future run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
red-knot Multi-file analysis & type inference testing Related to testing Ruff itself
Projects
None yet
Development

No branches or pull requests

4 participants