-
Notifications
You must be signed in to change notification settings - Fork 409
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
Fix inline_tests action when partition list is empty #8849
Conversation
5c84f47
to
626704c
Compare
|> Action.diff ~optional:true fn) | ||
|> Action.concurrent | ||
in | ||
Action.Full.make ~sandbox @@ Action.progn [ run_tests; diffs ])) |
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.
Doesn't have to be in this PR, but wouldn't it make more sense to concurrently run and diff rather than concurrently run and then concurrently diff? At the moment, all the diffs have to wait for all the tests to run.
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.
There is nothing allowing us to know what diff to perform after a given run
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.
Oh yes of course.
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.
Fix looks good to me. Of course, we wouldn't need it if Action.Full.make
was smart enough to avoid sandboxing if an action doesn't need it rather than complaining.
Thanks. a CHANGES entry would be appropriate |
done |
DCO please :) |
86a8338
to
6cd402b
Compare
Signed-off-by: Hugo Heuzard <[email protected]>
Signed-off-by: Hugo Heuzard <[email protected]>
6cd402b
to
35d821c
Compare
* Parallel inline_tests: expose bug with empty partition list Signed-off-by: Hugo Heuzard <[email protected]> * Parallel inline_tests: Fix when partition list is empty Signed-off-by: Hugo Heuzard <[email protected]> --------- Signed-off-by: Hugo Heuzard <[email protected]> Co-authored-by: Hugo Heuzard <[email protected]>
CHANGES: - Fix `dune rpc` commands on Windows (ocaml/dune#8806, fixes ocaml/dune#8799, @nojb) - Fix `inline_tests` when the partition list is empty (ocaml/dune#8849, fixes ocaml/dune#8848, @hhugo)
CHANGES: - Fix `dune rpc` commands on Windows (ocaml/dune#8806, fixes ocaml/dune#8799, @nojb) - Fix `inline_tests` when the partition list is empty (ocaml/dune#8849, fixes ocaml/dune#8848, @hhugo)
fix #8848
This is a 3 lines fix if your ignore white space