Skip to content
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: output non-zero code when devenv test fails #1510

Merged
merged 3 commits into from
Oct 9, 2024

Conversation

sandydoo
Copy link
Member

@sandydoo sandydoo commented Oct 8, 2024

Make sure we don't exit with code 0 if devenv test fails.

This requires both a module and a CLI update. On older versions of devenv, the updated tasks runner will cut of a bit of output at the end ("Tests failed :("), as it bails out of the nix develop command early.

I would've preferred something like cmd.run().await?.bail_on_error(), where we can make the decision to bail on each invocation, but that's a bit too invasive at this time.

Reported on Discord.

@sandydoo sandydoo added bug Something isn't working cli Related to the devenv CLI labels Oct 8, 2024
@sandydoo
Copy link
Member Author

sandydoo commented Oct 8, 2024

Nice! Caught some test errors.

Copy link

cloudflare-workers-and-pages bot commented Oct 8, 2024

Deploying devenv with  Cloudflare Pages  Cloudflare Pages

Latest commit: 497332c
Status: ✅  Deploy successful!
Preview URL: https://4cd46af8.devenv.pages.dev
Branch Preview URL: https://propagate-task-failure.devenv.pages.dev

View logs

@sandydoo
Copy link
Member Author

sandydoo commented Oct 8, 2024

Hmmm can't get it to fail locally 🤔

@sandydoo
Copy link
Member Author

sandydoo commented Oct 8, 2024

✔ Building shell in 0.0s.
/nix/store/s5mr3j8hj6ph3ppx03q46r6jjdldbkfg-init-npm.sh: line 18: /nix/store/230hxd4jaz9rdk20dh66na4bx26c10fc-nodejs-slim-20.11.1/bin/npm: No such file or directory
/nix/store/s5mr3j8hj6ph3ppx03q46r6jjdldbkfg-init-npm.sh: line 18: /nix/store/230hxd4jaz9rdk20dh66na4bx26c10fc-nodejs-slim-20.11.1/bin/npm: No such file or directory

@sandydoo
Copy link
Member Author

sandydoo commented Oct 8, 2024

Depends on #1511.

@domenkozar domenkozar marked this pull request as ready for review October 9, 2024 05:47
@domenkozar domenkozar merged commit e7a0bc5 into main Oct 9, 2024
257 of 273 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cli Related to the devenv CLI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants