search

holmgr / cargo-sweep

A cargo subcommand for cleaning up unused build files generated by Cargo
MIT License
693 stars 31 forks source link

Use error from stderr if stdout is empty #63

Closed adumbidiot closed 1 year ago

adumbidiot commented 2 years ago

I believe this fixes #24.

fenhl commented 2 years ago

Indeed it does.

adumbidiot commented 1 year ago

Honestly, I wasn't sure why stdout is involved at all when it seems the stderr always has the error (from my testing). However, I assumed that this isn't always the case, as the person who wrote this block probably got an error from the stdout since thats the way they wrote that. Also, I am not sure how I would write a test for this. Would we somehow create a project with an error for sweep to find?

jyn514 commented 1 year ago

Would we somehow create a project with an error for sweep to find?

That was my idea, yeah :) You should be able to reproduce it consistently by adding a custom rustc shell script in PATH that always gives an error.

It looks like cargo-sweep only has unit tests currently, not integration tests, so we'd need to set those up - in deadlinks I use assert_cmd which works pretty well without much boilerplate: https://github.com/deadlinks/cargo-deadlinks/blob/73de1dfcc24c6dcb2743505c176ff96aff9ca28d/tests/broken_links.rs#L8-L28 Let me know if that sounds like a hassle and I can add this first test, and hopefully it will be easier to add more in the future :)

adumbidiot commented 1 year ago

Sorry, not sure if I have the time for that in the next few weeks. I'll try to implement that though if this test is still missing when I get some free time.

fenhl commented 1 year ago

Could we get a crates.io release with this fix please?