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

Fragile test case #870

Closed
vbgl opened this issue Dec 9, 2024 · 13 comments
Closed

Fragile test case #870

vbgl opened this issue Dec 9, 2024 · 13 comments

Comments

@vbgl
Copy link

vbgl commented Dec 9, 2024

The cram test in test/blackbox-tests is prone to spurious failures. In various places, it blindly calls dune and expects no output from it. For instance:

$ dune build @melange

However, dune may print warning messages:

Warning: Cache directories could not be created: Permission denied; disabling cache

@davesnx
Copy link
Member

davesnx commented Dec 9, 2024

It seems related with the cache. Disabling the cache for this call is probably a good idea. Thanks

@davesnx
Copy link
Member

davesnx commented Dec 9, 2024

I have tried enabling the cache and running the tests and I don't see this warning. What version of dune are you using?

@vbgl
Copy link
Author

vbgl commented Dec 9, 2024

Version 3.17.0.

@davesnx
Copy link
Member

davesnx commented Feb 24, 2025

does --display=quiet fixes the issue for you?

@vbgl
Copy link
Author

vbgl commented Feb 24, 2025

No

File "test/blackbox-tests/useCallback.t", line 1, characters 0-0:
(cd _build/.sandbox/d4afbebc44abb8b11405c08dd446ed7a/default && /nix/store/lvj768zc0vqkzal5frxhc1yk6kp851bk-diffutils-3.10/bin/diff -u test/blackbox-tests/useCallback.t test/blackbox-tests/useCallback.t.corrected)
--- test/blackbox-tests/useCallback.t   2025-02-24 13:54:49.560315645 +0000
+++ test/blackbox-tests/useCallback.t.corrected 2025-02-24 13:54:49.560315645 +0000
@@ -17,6 +17,10 @@
   > EOF

   $ dune build --display=quiet @melange
+  Warning: Cache directories could not be created: Permission denied; disabling
+  cache
+  Hint: Make sure the directory /homeless-shelter/.cache/dune/db/temp can be
+  created
   $ cat _build/default/js-out/x.js
   // Generated by Melange

@davesnx
Copy link
Member

davesnx commented Feb 24, 2025

It looks like an issue with your setup, and not particularly with our cram tests. I'm up for making the test more resilient to any setup, but I'm unsure if dune build --cache=disabled or a permission issue appears in your system.

Would you play with it and try to open a PR?

@anmonteiro
Copy link
Member

This is more likely a bug in Nix's buildDunePackage. I fixed it for the nix-overlays in nix-ocaml/nix-overlays#1908, perhaps upstream should consider doing the same.

@vbgl
Copy link
Author

vbgl commented Feb 25, 2025

I disagree. This particular test line asserts: “running dune will produce no output”. However, the nominal behavior of dune, when cache is enabled and does not work is to output a warning. So I think this assertion is wrong.

If you want to reproduce the issue locally, you can try to configure the dune cache:

export DUNE_CACHE_ROOT=/path/that/does/not/exist/

However, the purpose of this test — I presume — is neither to test dune nor to test whether it is sensibly configured. So maybe it is just better to remove this wrong assertion. One possible way to do it is to kill the output:

$ dune build @melange 2>/dev/null

@davesnx
Copy link
Member

davesnx commented Feb 25, 2025

If the dune build fails, we don't want to hide the errors and want to see the output. Hiding it to dev/null or removing it by grep warnings/hints isn't a great idea either. That's why I thought a flag would hide the annoyance.

@davesnx
Copy link
Member

davesnx commented Feb 25, 2025

For now, I ensure the env var isn't set and remove the fragility of the test here: fc22f01

@davesnx
Copy link
Member

davesnx commented Feb 25, 2025

I opened the issue in dune in case they know something I'm missing (ocaml/dune#11501)

Thanks for the issue, closing

@davesnx davesnx closed this as completed Feb 25, 2025
@vbgl
Copy link
Author

vbgl commented Feb 25, 2025

This is not going to work. You have just removed a way to reproduce the issue. And you have badly messed up with the configuration of the dune cache…

@davesnx
Copy link
Member

davesnx commented Feb 26, 2025

What kind of setup do you use to run the tests and have DUNE_ROOT_PATH pointing to a non existing dir?

If you know a better way to hide the message (which is not very relevant to enable cache for a tiny test inside the sandbox) and run dune without ignoring compiler errors, please push a PR with the fix. Meanwhile, this is the best I can do for a fragile test.

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

No branches or pull requests

3 participants