-
Notifications
You must be signed in to change notification settings - Fork 349
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
[nix] tests are failing/flakey #2103
Comments
note, while I was leaving open the chance for flakiness, I haven't actually seen these be flakey, just consistently failing. |
Odd... pinning a few older revisions isn't helping, and then the blame via GitHub UI doesn't indicate changes to that test function recently. |
This is odd, I do Looking at it being the classic generic "Input/output error" from the OS, maybe you have some exotic/broken filesystem or something?. Basically the sandbox didn't work 😄 |
Yeah, it's bizarre. I've tried it on two different machines now, albeit they're practically identical. My root is zfs, my tmp is tmpfs. Not sure... I might file a nixpkgs bug. |
In the issue I filed on Nix, someone else said they were able to build. I'm building it in a loop.... sometimes only one test fails, but I haven't had a pass yet (even though it should be building on a single core with the settings I'm using). |
I think the tests are flakey. I finally got it to build:
Finally built it. |
The tests do a recursive file system copy of some repos they create. They do that by walking the file system. Could it be that you have something running in the background that notices these temporary directories and adds a temporary file in them, which the test might then fail to copy if the background process had already deleted it. |
Not to hammer the point too hard, but I had dozens of failures before setting it to a single core, and then it immediately built. |
Since you're able to reproduce it quite reliably, could you try applying this and see what it reports? I'm curious to see if it's a particular path every time. diff --git a/lib/tests/test_bad_locking.rs b/lib/tests/test_bad_locking.rs
index 68ac86ebc9...0f31ee743a 100644
--- a/lib/tests/test_bad_locking.rs
+++ b/lib/tests/test_bad_locking.rs
@@ -45,7 +45,9 @@
if child_left.is_dir() {
sub_dirs.push(base_name.to_os_string());
} else {
- std::fs::copy(&child_left, child_output).unwrap();
+ if let Some(err) = std::fs::copy(&child_left, &child_output).err() {
+ panic!("failed to copy {child_left:?} to {child_output:?}: {err}");
+ }
}
}
// Walk the base and find files removed in the right side, then remove them in
@@ -75,7 +77,9 @@
} else if !child_base.exists() {
// This overwrites the left side if that's been written. That's fine, since the
// point of the test is that it should be okay for either side to win.
- std::fs::copy(&child_right, child_output).unwrap();
+ if let Some(err) = std::fs::copy(&child_right, &child_output).err() {
+ panic!("failed to copy {child_right:?} to {child_output:?}: {err}");
+ }
}
}
// Do the merge in subdirectories |
Here's what I'm testing:
(so default multi-core-ish behavior)
Let me know if you want me to script this to get pass/fail rate, or make a log of the fail cases, or if this is sufficient. |
I think that's sufficient, thanks. I still have no idea why it would sometimes fail to copy some files, however. If you're curious enough, I suppose you could reduce the test case to just manually build up some random directories and see if running the same |
I ran into some issues here when switching our tests to use `.jj`-internal git repos. For example, the `std::fs::copy()` calls started failing, which may be related to #2103. I think one problem is that we could end up calling `merge_directories()` twice for the same directory. This patch fixes that by deduping the paths we call with, and makes the function assume that the output directory doesn't exist.
I ran into some issues here when switching our tests to use `.jj`-internal git repos. For example, the `std::fs::copy()` calls started failing, which may be related to #2103. I think one problem is that we could end up calling `merge_directories()` twice for the same directory. This patch fixes that by deduping the paths we call with, and makes the function assume that the output directory doesn't exist.
I ran into some issues here when switching our tests to use `.jj`-internal git repos. For example, the `std::fs::copy()` calls started failing, which may be related to #2103. I think one problem is that we could end up calling `merge_directories()` twice for the same directory. This patch fixes that by deduping the paths we call with, and makes the function assume that the output directory doesn't exist.
I ran into some issues here when switching our tests to use `.jj`-internal git repos. For example, the `std::fs::copy()` calls started failing, which may be related to #2103. I think one problem is that we could end up calling `merge_directories()` twice for the same directory. This patch fixes that by deduping the paths we call with, and makes the function assume that the output directory doesn't exist.
I ran into some issues here when switching our tests to use `.jj`-internal git repos. For example, the `std::fs::copy()` calls started failing, which may be related to jj-vcs#2103. I think one problem is that we could end up calling `merge_directories()` twice for the same directory. This patch fixes that by deduping the paths we call with, and makes the function assume that the output directory doesn't exist.
Description
Hi! Hope all is well.
jj
is not building with nix right now, not sure if its a sandbox thing, guessing from the test names?Steps to Reproduce the Problem
nix build github:martinvonz/jj
(ornix build github:martinvonz/jj/7ad2270c0
)Expected Behavior
It builds.
Actual Behavior
It fails when testing:
fuller log:
Specifications
(Though this shouldn't matter, I don't override jj's nixpkgs)
The text was updated successfully, but these errors were encountered: