codecov should be relaxed for the tools directory

[wacban]

The code coverage requirements in the CI should be relaxed for the tools directory and potentially some other non-critical directories. I would suggest setting the target coverage level to 0% or disabling it for those directories.

[wacban]

@nagisa @Ekleog-NEAR

[Ekleog-NEAR]

Just to be sure I understand, what do you call "code coverage requirements"? AFAICT there are no requirements set up yet, and code coverage analysis is purely informational, is it not?

[wacban]

You're correct in that it's non-blocking but if it fails it still shows up as a red, failed job on the PR. It's personal but it's really frustrating to me. I strongy believe we should not get devs used to the red colour or they will just start ignoring all failing jobs.

[Ekleog-NEAR]

Got it :)

I totally agree with you, and it’s actually part of the reason for #10218: to make the red in code coverage failures to actually appear 😅

So now that I’m clear on the need, I’m curious: is the tools/ folder not expected to be used in production for devops by node operators?

If I understand the codecov defaults properly, the current setting is (project-wide) "coverage does not decrease, and the patch coverage is at least the project-wide coverage". Assuming the tools/ folder is indeed supposed to be used by node operators, it sounds like a good default to me, though I do think that we should tighten the requirements for folders other than tools/: we’re never going to have to work to increase coverage, and we can land whatever we want if we need to by ignoring the red cross, but we should most of the time not decrease coverage in a patch. Maybe we should just disable the second part of the current setting, and just require that overall coverage does not decrease? Does that make sense?

[wacban]

As far as I know the vast majority of /tools is not intended to be used in production by node operators. I'm pretty sure there are exceptions though and it would be good to check with the node team but I'm not sure how to tag them here.

I don't have a strong opion and I'd rather rely on your expertise to come up with the best solution. Anything like selectively setting the threshold to zero for some directories, or allowing the developer to add an exception for some more nested subdirectory or just reporting without red failing jobs is fine for me.

[Ekleog-NEAR]

@nikurt I know you care a lot about developer experience (and thank you so much for that ❤️). Do you have any input on this? :)

[nikurt]

I agree that tools/ are not intended for use in production.
Maybe undo-block can be used by node operators in an emergency.
Other tools are mostly for debug or dev purposes.

Therefore it should be fine to have no code coverage requirement for the code in tools/.

I can make a case for requiring basic correctness tests for all the tools, though. That is unrelated to @wacban 's original question.

[wacban]

While we're on the topic I also think we should relax the project diff coverage a tiny bit. The reason is that adding a line of documentation and potentially a line of comment just increases the total number of lines while the number of covered lines stays the same and the coverage effectively decreases. E.g. #10303 is read because coverage dropped by 0.02%. Let me know if this makes sense and I'll be happy to write a quick PR for this.

[nagisa]

The true solution would be to consider SLOCs only – I would be surprised if that isn’t the case currently...

But I also agree that being super strict on coverage decreases is not particularly productive given our current workflows and development style.