Generated sys tests: Better command error handling #1499
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The generated tests for `sys` crates sometimes call `Command::output`. It is used to find CFLAGS arguments for tested packages and run the executables created from `constant.c` and `layout.c` tests. `Command::output`, by default, captures `stdout` and `stderr`. This means that, by default, no feedback for why the tests fail will be printed. The only information shown to the end-user is the command that was ran. You could copy/paste and run yourself the command printed in the error message, but it might not be evident as a "next step" for users. This is unlike the compilation command, which uses `cmd.spawn()?.wait()?`. We can see compilation error messages, but not pkg-config error messages. To solve this, we: 1. Inherit `stderr` from the parent descriptor. This will print the command's error message in the terminal. 2. Add the command's `stdout` to the error message returned by `pkg_config_cflags` and `get_c_output`. Now. Errors generated by the spawned process are visible to the end user, which makes test errors more actionables. The output is included for exhaustiveness. It may not be useful, but it makes sense to give to the user all the information necessary.
sdroege
reviewed
Aug 14, 2023
sdroege
reviewed
Aug 14, 2023
sdroege
approved these changes
Aug 16, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The generated tests for
syscrates sometimes callCommand::output. It is used to find CFLAGS arguments for tested packages and run the executables created fromconstant.candlayout.ctests.Command::output, by default, capturesstdoutandstderr. This means that, by default, no feedback for why the tests fail will be printed. The only information shown to the end-user is the command that was ran.You could copy/paste and run yourself the command printed in the error message, but it might not be evident as a "next step" for users.
This is unlike the compilation command, which uses
cmd.spawn()?.wait()?. We can see compilation error messages, but not pkg-config error messages.To solve this, we:
stderrfrom the parent descriptor. This will print the command's error message in the terminal.stdoutto the error message returned bypkg_config_cflagsandget_c_output.Now. Errors generated by the spawned process are visible to the end user, which makes test errors more actionables.
The output is included for exhaustiveness. It may not be useful, but it makes sense to give to the user all the information necessary.