Skip to content

fix: output was silenced on task fail with keep-order#5175

Merged
jdx merged 2 commits into
jdx:mainfrom
artemisart:fix/keep-order-output-on-failure
May 25, 2025
Merged

fix: output was silenced on task fail with keep-order#5175
jdx merged 2 commits into
jdx:mainfrom
artemisart:fix/keep-order-output-on-failure

Conversation

@artemisart

@artemisart artemisart commented May 25, 2025

Copy link
Copy Markdown
Contributor

I moved the keep-order output before the exit on fail so that we can see the task outputs.
I also added the task timings for --output timed and keep-order + added final timing for all output types (splitting fn timings() in timings() + task_timings()) is that ok?
Fixes #5077 #5078

Unfortunately the e2e test is flaky, sometimes I get:

$ mise run -o keep-order all
E2E assertion failed: [mise run -o keep-order all] expected '[a] a
[b] b' but got '[b] $ echo b ; exit 1
[a] $ echo a
[b] b
Finished in 70.3ms
[b] ERROR task failed'
tasks/test_task_keep_order: 3s
E2E test tasks/test_task_keep_order failed: exited with status code 1

And sometimes everything is fine (the difference is outputing or not the commandlines executed). I don't get why I have this error here and not on the other tests.

@artemisart artemisart force-pushed the fix/keep-order-output-on-failure branch from 5b872fd to ebceea8 Compare May 25, 2025 10:38
@artemisart artemisart marked this pull request as ready for review May 25, 2025 10:38
@artemisart

artemisart commented May 25, 2025

Copy link
Copy Markdown
Contributor Author

Hah some idea on the flaky test: assert() check part of the output, however the order is not guaranted when running parallel tasks so if the output of a task is displayed before the commandline of another one, the assert fail because the output of all tasks is not continuous.

EDIT: better guess, task b in my test can fail before a has started to run, so it's stopped and we have no output from a. Sorry I missed this, will add another PR.

@jdx jdx merged commit f47bd4f into jdx:main May 25, 2025
17 checks passed
jdx pushed a commit that referenced this pull request May 25, 2025
Fix flaky test introduced in #5175 (sorry!) where task b can fail before
task a has a chance to run so a has no output.
If you'd like a more precise test I can add a `assert_fail_matches()` in
[e2e/assert.sh](e2e/assert.sh).
@artemisart artemisart deleted the fix/keep-order-output-on-failure branch May 25, 2025 13:09
jdx pushed a commit that referenced this pull request May 26, 2025
### 🐛 Bug Fixes

- output was silenced on task fail with keep-order by
[@artemisart](https://github.com/artemisart) in
[#5175](#5175)
- avoid mapfile to run e2e tests on macOS (bash 3.2) by
[@artemisart](https://github.com/artemisart) in
[#5170](#5170)
- flaky keep-order e2e test by
[@artemisart](https://github.com/artemisart) in
[#5178](#5178)
- watch mise.lock for changes by [@jdx](https://github.com/jdx) in
[#5184](#5184)
- remote task dependency does not work by
[@roele](https://github.com/roele) in
[#5183](#5183)
- rayon -> tokio by [@jdx](https://github.com/jdx) in
[#5172](#5172)
- cache results from version host by [@jdx](https://github.com/jdx) in
[#5187](#5187)
- cache results from version host for aqua packages by
[@jdx](https://github.com/jdx) in
[#5188](#5188)

### 📚 Documentation

- standardize subcommand format to 'u|use' for consistency by
[@LuckyWindsck](https://github.com/LuckyWindsck) in
[#5167](#5167)
- clarify how to enable ideomatic version file reading for ruby by
[@amkisko](https://github.com/amkisko) in
[#5163](#5163)

### 🧪 Testing

- added perf test by [@jdx](https://github.com/jdx) in
[#5179](#5179)
- skip benchmark errors for now by [@jdx](https://github.com/jdx) in
[#5186](#5186)

### Chore

- fix clippy issue in xonsh by [@jdx](https://github.com/jdx) in
[#5180](#5180)
- improve shfmt linter by [@jdx](https://github.com/jdx) in
[#5181](#5181)
- cargo up by [@jdx](https://github.com/jdx) in
[3ece604](3ece604)
- fix hyperfine step summary by [@jdx](https://github.com/jdx) in
[36ab4a1](36ab4a1)
- adjust perf thresholds by [@jdx](https://github.com/jdx) in
[4113a3b](4113a3b)

### New Contributors

- @amkisko made their first contribution in
[#5163](#5163)
- @LuckyWindsck made their first contribution in
[#5167](#5167)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants