Cod 1815 - Add the full command command that was run to the benchmark uri#191
Conversation
not-matthias
left a comment
not-matthias
left a comment
There was a problem hiding this comment.
I think we can improve the testing a bit, to avoid re-implementing the same set of shell tests in every new executor (lmk if there are any technical reasons that prevent this for the exec-harness).
Other than that, it looks pretty good. Next review should be quick :)
| } | ||
|
|
||
| /// Test that a command with shell operators (&&) works correctly when passed as a single argument | ||
| #[test] |
There was a problem hiding this comment.
We also have the executor tests, which already include a lot of these edge cases: https://github.com/CodSpeedHQ/runner/blob/0cfb188332fa11041e65245f2a3cbe69ee3154ff/src/executor/tests.rs#L14-L50
We should test it there, so that we can reuse the existing test cases that ensure that the same shell scripts work across all executors.
There was a problem hiding this comment.
As discussed, I added a small subset of commands to run with the exec-harness. For now, script-like multi-commands are kinda out of scope, we'll see if we get user demand.
Kept the integration tests in the exec-harness, we'll see if we need more integration tests with pipes and whatnot form the codspeed runner
GuillaumeLagrange
force-pushed
the
cod-1815-add-the-command-that-was-run-to-the-upload-payload-and
branch
2 times, most recently
from
6e42e0f to
7b1c4ca
Compare
Currently, the codspeed rust core library automatically sends the integration metadata message to the runner. This will be changed later, but in the meantime, remove the second call.
GuillaumeLagrange
force-pushed
the
cod-1815-add-the-command-that-was-run-to-the-upload-payload-and
branch
from
7b1c4ca to
7a24f1d
Compare
GuillaumeLagrange
deleted the
cod-1815-add-the-command-that-was-run-to-the-upload-payload-and
branch
2 participants
No description provided.