-
Notifications
You must be signed in to change notification settings - Fork 4
feat(runner): distinguish instrumented command and tool error in codspeed #67
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
feat(runner): distinguish instrumented command and tool error in codspeed #67
Conversation
74043ce
to
f35cac9
Compare
f35cac9
to
be2db7e
Compare
Depends on CodSpeedHQ/codspeed-rust#86 The correct exit code is now displayed (illegal instruction = signal 4)
|
Is there an easy way for us to write a test of this behavior? At least testing the script isolated from the rest |
479fc5c
to
1d94040
Compare
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.
I'm not super confident merging this right now, mostly because of the quoting and wrapping of parameters.
In
cmd.args(["sh", "-c", &run_cmd]);
I think we should replace sh -c
with our wrapper script directly. I think it would avoid messing with the quotes and formatting command as strings.
1d94040
to
5668cde
Compare
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.
just one last thing to simplify the script and I think we're good to go
5668cde
to
1500c44
Compare
1500c44
to
dda455a
Compare
dda455a
to
998b2e3
Compare
Added a small wrapper script, which executes
cargo codspeed run
in a subprocess and then writes the status code to a file.Logs when the benchmark failed (in this case with a
panic
):Logs when valgrind error is detected (only an example, this doesn't cause valgrind to crash)