tarpaulin skip no longer working since 0.13.4

[Licenser]

See: https://github.com/wayfair-tremor/tremor-runtime/pull/321/checks?check_run_id=803402444

This is seems to be a regression since 0.13.3

   Compiling value-trait v0.1.10
error: could not compile `value-trait`.

To learn more, run the command again with --verbose.
warning: build failed, waiting for other jobs to finish...
error: build failed
[ERROR tarpaulin] Failed to compile tests! Error: cannot find attribute `skip` in this scope
Error: "Failed to compile tests! Error: cannot find attribute `skip` in this scope"
##[error]The process '/usr/share/rust/.cargo/bin/cargo' failed with exit code 1

[xd009642]

Yeah it's mentioned in #486 I didn't expect that effect but that's largely because I didn't fully grok cfg_attr and now it's clear I was kind of misusing it.

I'll work on this after my working day is finished but the current thinking is that it might be necessary to add:

#![feature(register_tool)]
#![register_tool(tarpaulin)]

// then replace #[cfg_attr(tarpaulin, skip)] with:
#[tarpaulin::skip]
fn my_function() {}

It's more idiomatic but it does involve using nightly rust if you want skip attributes. Alternatively, I could remove --cfg=tarpaulin from the RUSTFLAGS and have that feature as an explicit opt-in for anyone who wants to change over. I'm not really 100% decided so open for feedback from users who it affects (i.e. you and @jonhoo )

[jonhoo]

Hmm, I don't think you should have to require nightly to support custom attributes? serde for example has them, and does not require nightly? Same with clippy, which uses the clippy:: prefix?

[xd009642]

Clippy and rustfmt are tool attributes supported in the compiler. I thnik serde is probably a proc-macro attribute so taking in a token stream and generating code etc? This https://doc.rust-lang.org/reference/attributes.html#tool-attributes and rust-lang/rust#66079 are my references for this though I might be incorrect or there might be an easier way to achieve what we want

[jonhoo]

Oh, interesting. Yeah, you're probably right on both accounts.. Hmm...
How does tarpaulin currently find the skip attribute? If it just does source-code analysis, could it be special comment or something instead?

// #[tarpaulin::skip]
fn my_function() {}

[xd009642]

it uses syn and parses object attributes looking for them, I can add code to find #[tarpaulin::skip] and it will compile in a crate with the feature gate and work, or can use the old way (which seems was how it was recommended to be done before tool attributes existed) and not pass --cfg=tarpaulin. So if --cfg=tarpaulin is opt in, that might be the most flexible solution then if people are using it they can just use #[tarpaulin::skip].

I could also see if the rust team are willing to give me a tool lint but I'm not sure if I'm big enough for that

[jonhoo]

Here's another option:

#[cfg(tarpaulin_skip)]

[xd009642]

oh true true, I kinda like that

[Licenser]

+1 to @jonhoo's suggestion not having to rely on feature and I think by that nightly is nice :D

[xd009642]

I've added support for both just pushed to a feature branch to make sure it passes CI. Also added an integration test for it (will also add another one for the nightly feature). Once it passes I'll do a release since it's a pretty small change and with the extra tests I can be more confident about it

[xd009642]

Wait #[cfg(tarpaulin_skip)] doesn't work for the aim because it conditionally compiles things out so it doesn't work to just ignore the blocks... However, #[cfg_attr(tarpaulin_skip,)] does but that makes me feel like I'm going around in circles a bit! Any thoughts from either of you?

[xd009642]

It's on the branch tarpaulin_skip and relevant integration tests are in tests/data/tarpaulin_attrs/ and tests/data/tool_attr I've also updated the readme so the examples should make it clear how I envisage it working

[xd009642]

I've also yanked 0.13.4 until this is sorted out although that doesn't help anyone affected using the latest docker image

[jonhoo]

Err, I guess it'd have to be more like #[cfg(not(tarpaulin_include))]?

[xd009642]

that still changes what code is compiled in when running in/out of tarpaulin, the aim of the initial implementation was so that tarpaulin would remove the lines from the results but they'd still be present and ran.

Which is why I now have in the branch tarpaulin_skip the options for stable #[cfg_attr(tarpaulin_skip,)], or nightly #[tarpaulin::skip] to just filter out code from results. And for conditionally compiling in code there's #[cfg(tarpaulin)] or #[cfg(not(tarpaulin))] and for ignoring tests in tarpaulin there's #[cfg_attr(tarpaulin, ignore)]

[jonhoo]

@xd009642 no, the above would never actually change anything, since it'd always be true

[xd009642]

ooh right so I wouldn't --cfg it I follow now. I suppose it is a unique enough name that someone wouldn't try to use it themselves 🤔

[xd009642]

Luckily enough this part of the code is quite easy to modify now so I've just made the change and pushed it, feel free to feedback on the readme and example projects

[jonhoo]

Yeah, my thinking is that you'd pass --cfg=tarpaulin, but not --cfg=tarpaulin_include. And then your processor would look for

#[cfg(not(tarpaulin_include))]

That way, people can still use #[cfg(tarpaulin)] for actually having compilation depend on tarpaulin, and they can use ^ for telling tarpaulin not to consider a particular item for coverage without affecting compilation on stable. It's not very pretty, but it does the job on stable 😅 And then you'd still recommend #[tarpaulin::skip] for anyone who's on nightly.

[xd009642]

Yeah I've just implemented it like that and tests pass and it's working as expected on a larger project I used it on. I'll merge it into develop, sleep on it and then tomorrow work out if it's ready for release. Don't want to jump the gun again 😅

[jonhoo]

It's still a break release of course, since the old cfg_attr version won't work, but I think this version is more robust.

[xd009642]

Yeah I was planning on making it 0.14.0

[Licenser]

Thanks for the super speedy reply and resolution @xd009642 <3!

[xd009642]

I've done a few more tests and now 0.14.0 release is in progress (once CI and docker hub is finished it will be all done). As always let me know if anything doesn't work with it 👍

[Stargateur]

$ cargo tarpaulin --verbose
[INFO tarpaulin] Creating config
[INFO tarpaulin] Running Tarpaulin
[INFO tarpaulin] Building project
Compiling time v0.2.16
error: could not compile time.

To learn more, run the command again with --verbose.
[ERROR tarpaulin] Failed to compile tests! Error: cannot find attribute skip in this scope
Error: "Failed to compile tests! Error: cannot find attribute skip in this scope"
$ cargo tarpaulin --version
cargo-tarpaulin version: 0.14.1

Still happen

[xd009642]

@Stargateur That's an issue in time, they've fixed it but haven't yet released a new version since then. Either see if they'll release a new version soon or in the meantime stick to tarpaulin 0.13.x