-
Notifications
You must be signed in to change notification settings - Fork 428
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
dune-pkg: Better error messages on extract fail #11560
Conversation
It appears that |
Thanks for the patch! Indeed different tar implementations will have different error messages, and also the location of tar on different systems will differ; there's no guarantee it will be at |
Looks like this also solves #11550. |
Yes, you can enable and disable tests depending on the platform they're running on: https://github.com/ocaml/dune/blob/main/test/blackbox-tests/test-cases/pkg/dune#L22-L26 but for cases where the errors are different I agree with @gridbugs that some postprocessing (or a fake injected |
This should now be passing, the issue on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good. The failing test is being flaky sometimes and fixes itself upon rerunning usually. I've added some comments of things to look into but overall I think this is a good approach.
Signed-off-by: Teague Hansen <[email protected]>
Signed-off-by: Teague Hansen <[email protected]>
Signed-off-by: Teague Hansen <[email protected]>
Signed-off-by: Teague Hansen <[email protected]>
Signed-off-by: Teague Hansen <[email protected]>
Signed-off-by: Teague Hansen <[email protected]>
Signed-off-by: Teague Hansen <[email protected]>
Signed-off-by: Teague Hansen <[email protected]>
Signed-off-by: Teague Hansen <[email protected]>
Signed-off-by: Teague Hansen <[email protected]>
Signed-off-by: Teague Hansen <[email protected]>
Signed-off-by: Teague Hansen <[email protected]>
d84a4ac
to
378814a
Compare
* add test cases for corrupted tar & tar.gz package source Signed-off-by: Teague Hansen <[email protected]> * add initial error message impl for failing extract on packages Signed-off-by: Teague Hansen <[email protected]> * clean up and format Signed-off-by: Teague Hansen <[email protected]> * add test for unzip and rename extract fail test case Signed-off-by: Teague Hansen <[email protected]> * ensure temporary file is destoryed by finalize Signed-off-by: Teague Hansen <[email protected]> * update test cases with better sed commands Signed-off-by: Teague Hansen <[email protected]> * switch from In_channel to Io Signed-off-by: Teague Hansen <[email protected]> * additional sanitization to test output for nondeterministic output Signed-off-by: Teague Hansen <[email protected]> * also sanatize unavailable-package-source.t Signed-off-by: Teague Hansen <[email protected]> * use User_message.command for command printing Signed-off-by: Teague Hansen <[email protected]> * add unzip and tar dependency for pkg-extract-fail.t Signed-off-by: Teague Hansen <[email protected]> * fix unavailable-package-source.t Signed-off-by: Teague Hansen <[email protected]> --------- Signed-off-by: Teague Hansen <[email protected]>
This pull request is in reference to and would close #11549 (as well as possibly other related issues) by capturing the output of commands such as
tar
,gzip
, andunzip
when dune attempts to install a package and reporting a corresponding error message. The following is an example of the tooltar
encountering a corrupted file:One note, relating to the original issue was that I was unable to test behavior when a corresponding tool did not exist on the system. If any guidance on how to check this case could be provided I would be more than willing to add additional handling and tests for this case :)