sagiegurari / cargo-make

Rust task runner and build tool.
https://sagiegurari.github.io/cargo-make/
Apache License 2.0
2.56k stars 125 forks source link

Fix --skip-init-end-tasks argument #1109

Closed 06393993 closed 2 months ago

06393993 commented 3 months ago

Fix #1108.

06393993 commented 3 months ago

It seems to me that all integration tests are ignored, so I didn't add any integration tests for this feature. Please let me know if I am missing anything. Thanks.

sagiegurari commented 2 months ago

tests are split to multi and single threaded. they are not ignored but you cant teat cargo make with cargo test. only via cargo make itself

06393993 commented 2 months ago

Added 2 integration tests. One thread safe, one not thread safe. Verified locally with:

sagiegurari commented 2 months ago

@06393993 looks good.

sagiegurari commented 2 months ago

@06393993 can you fix the tests please? also moved it to a new dev branch

06393993 commented 2 months ago

~I figured out the failure: for not thread-safe tests, we are setting the global logger in the new test, which may causes other tests to panic on error!. The fix is to reset the global logger after the newly added test completes.~

EDIT: actually I incorrectly use "test2" instead of the correct "task2" in the failed execution_plan::execution_plan_test::create_task_extends_empty_env_bug_verification test.

06393993 commented 2 months ago

I have run cargo run --bin cargo-make --target-dir target/ci -- make ci-flow locally on my machine to make sure it passes the currently failed github action step.

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 67.22689% with 78 lines in your changes missing coverage. Please review.

Project coverage is 69.27%. Comparing base (e3e93de) to head (4dfc585).

Files Patch % Lines
src/lib/runner_test.rs 0.00% 49 Missing :warning:
src/lib/execution_plan_test.rs 83.63% 9 Missing :warning:
src/lib/cli_commands/print_steps_test.rs 85.71% 7 Missing :warning:
src/lib/execution_plan.rs 87.50% 6 Missing :warning:
src/lib/cli.rs 0.00% 5 Missing :warning:
src/lib/cli_commands/print_steps.rs 85.71% 2 Missing :warning:

:exclamation: There is a different number of reports uploaded between BASE (e3e93de) and HEAD (4dfc585). Click for more details.

HEAD has 3 uploads less than BASE | Flag | BASE (e3e93de) | HEAD (4dfc585) | |------|------|------| ||4|1|
Additional details and impacted files ```diff @@ Coverage Diff @@ ## 0.37.14 #1109 +/- ## ============================================ - Coverage 92.75% 69.27% -23.49% ============================================ Files 118 118 Lines 25961 26049 +88 ============================================ - Hits 24081 18046 -6035 - Misses 1880 8003 +6123 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sagiegurari commented 2 months ago

@06393993 merging. thanks a lot. i hope i'll release it this soon