search

xd009642 / tarpaulin

A code coverage tool for Rust projects
https://crates.io/crates/cargo-tarpaulin
Apache License 2.0
2.5k stars 180 forks source link

`skip-clean` and `force-clean` config options not working #963

Closed PyroTechniac closed 2 years ago

PyroTechniac commented 2 years ago

Describe the bug cargo tarpaulin doesn't respect the skip-clean and force-clean config options

Secondary bug: when calling cargo tarpaulin --skip-clean --force-clean a warning is thrown up about option incompatibility, but if both force-clean = true and skip-clean = true are set in a config file, no warning is emitted.

To Reproduce

[all-features-coverage]
workspace = true
all-features = true
ignore-tests = true
skip-clean = true
force-clean = false

[report]
out = ["Html", "Xml"]

then call cargo tarpaulin twice in a row, it should clean the project each time.

Expected behavior No cleaning of the project should occur.

xd009642 commented 2 years ago

So I realise why the warning is omitted. The Config struct has default values and just using serde to deserialize I can only spot non-default settings for skip-clean and force-clean. My config handling code looks for one of them being non-default and prioritises it. Whereas the clap code does some checks on what flags are provided and warns even if one of them is set as default.

However, I have tried this with just skip-clean = true in the config and it worked, with force-clean = false it didn't :thinking:. I'll do some more digging, but just putting skip-clean = true should work for you... If it doesn't if you have an example project I'll give that a try

xd009642 commented 2 years ago

Ah I did get force-clean=false in the tarpaulin.toml to be respected. https://github.com/xd009642/tarpaulin/pull/964 so that's part of the issue solved. Just curious on how it works on your stuff :thinking:

PyroTechniac commented 2 years ago

Once I'm off work tonight I'll try to create a working example and send it over

xd009642 commented 2 years ago

Have you had a chance to try it out? No rush but I'll probably merge the PR in a week or so if I don't hear back as it seems to fix one UX issue :eyes:

xd009642 commented 2 years ago

I've released merged the PR with the potential fix for this in, and 0.20.0 is in the process of releasing (once CI finished). So closing this issue, if it's not fixed though feel free to reopen

PyroTechniac commented 2 years ago

This seems to be working as intended now! Sorry for being unable to test this, and thank you for the quick fix!