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

fix: Make the dune trim test reproducible #10397

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Leonidas-from-XIV
Copy link
Collaborator

Since a while I have been reliably running into a this test failure:

File "test/blackbox-tests/test-cases/dune-cache/trim.t", line 1, characters 0-0:
diff --git a/_build/.sandbox/5f36f67e5370f082210438d2f14adc1a/default/test/blackbox-tests/test-cases/dune-cache/trim.t b/_build/.sandbox/5f36f67e5370f082210438d2f14adc1a/default/test/blackbox-tests/test-cases/dune-cache/trim.t.corrected
index dcda72696..87298ed0d 100644
--- a/_build/.sandbox/5f36f67e5370f082210438d2f14adc1a/default/test/blackbox-tests/test-cases/dune-cache/trim.t
+++ b/_build/.sandbox/5f36f67e5370f082210438d2f14adc1a/default/test/blackbox-tests/test-cases/dune-cache/trim.t.corrected
@@ -160,9 +160,9 @@ by the existence of [beacon_b].
   $ dune_cmd stat hardlinks _build/default/target_b
   2
   $ dune_cmd exists _build/default/beacon_a
-  false
-  $ dune_cmd exists _build/default/beacon_b
   true
+  $ dune_cmd exists _build/default/beacon_b
+  false

Weirdly enough it doesn't seem to be reproducible on CI or other people's machines (not even my old one before).

However the description why it was supposed to work seemed strange to me as it claimed that deleting the files in order would make the ctime older, but deleted files do not have a ctime. My hunch is that the order of deletion does not make a difference and the only thing that differs is the order in which the targets are built, so I've changed the test to make sure the order of building the targets is well-defined (and the FS gets time to update the timestamps in between so we get to reliably measure a difference).

Relying on the ctime of a file that was deleted makes little sense, so
this test makes sure that target_b is older by building it first.

Signed-off-by: Marek Kubica <marek@tarides.com>
@Leonidas-from-XIV
Copy link
Collaborator Author

Looks like I made it work on my machine and fail on CI. Needs more investigation what is going wrong.

@Leonidas-from-XIV Leonidas-from-XIV marked this pull request as draft April 8, 2024 14:08
@Leonidas-from-XIV
Copy link
Collaborator Author

After some analysis it seems like the problem I have locally is that I am running the test on btrfs and since a few Linux kernel releases the ctime is not updated on unlink, thus the wrong file gets deleted. Reported a bug to Fedora, will keep an eye on the situation.

@emillon
Copy link
Collaborator

emillon commented Apr 16, 2024

That's not ideal, but a solution in that could work in the meantime is to change the test to touch the other side of the hardlink after unlinking it (with a note explaining why, of course)

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

2 participants