"Coverage 100%" badge is misleading

[mrazauskas]

While trying to migrate a larger code base to Poku, I bump into an issue with async afterEach() and async test(). Moving code from afterEach() to the end of test() makes all work as expected. (I will create reproduction later.)

Out of curiosity I looked at: src/modules/helpers/test.ts. If I get it right, the test() helper logic lives here. Unfortunately the promise handling logic is wrapped with c8 ignore comments. Hm.. Might be I miss something, but at first glance it does not like these lines are not reachable.

"Coverage 100%" badge looks beautiful, but now for me it feels misleading. The high coverage rate was one of the aspects why I decided to migrate to Poku. Please consider updating the badge with actual coverage information.

[wellwelwel]

Hi, @mrazauskas.

The only mention for this is in Contributing.md:

poku/CONTRIBUTING.md

Lines 116 to 118 in 2b1eff0

	> [!tip]
	>
	> Don't be intimidated by high coverage, methods that vary according to platform, platform versions, _OS_ and processes _(`process.exit`, `process.once`, etc.)_ aren't tested against the coverage rate 🙋🏻‍♂️

"Don't be intimidated by high coverage, methods that vary according to platform, platform versions, OS and processes (process.exit, process.once, etc.) aren't tested against the coverage rate 🙋🏻‍♂️"

---

I use two specific comments related to a problem between c8 and tsx:

/* c8 ignore next */ // ?

• For some reason, the first line of some files and the last export always appear as zero coverage, including when the line is a comment.

/* c8 ignore next */ // Types

• When using overloads or importing types (ts), tsx build removes it before running, causing coverage mistakes.

---

Except by these two, all other comments should fit on process, Windows, previous versions from Node.js or vary for Bun and Deno platforms.

That line you mentioned, there's really no point in ignoring it in the coverage. I apologize for any misunderstanding and would like to hear your opinion on these exceptions I mentioned. 100% coverage is indeed Poku's goal.

---

Being honest, I loved the nudge and will revisit all the c8 comments, thank you for that.

Also, I'm considering migrating from c8 to monocart-coverage-reports.

[mrazauskas]

All is fine. This is only a badge (;

By the way, I was thinking to recommend looking at monocart-coverage-reports. It is really good in filtering out type only or empty lines. And the native v8 / codecov coverage is their strong side.

[mrazauskas]

By the way, those c8 ignore comments are shipped to npm and increase the install size without being anyhow useful. Just another argument (;

[wellwelwel]

By the way, those c8 ignore comments are shipped to npm and increase the install size without being anyhow useful. Just another argument (;

Yes, it bothers me a lot.

I tried a test locally with monocart-coverage-reports and only 8 comments remained 🎉
Most due to process, for example:

/* c8 ignore next 3 */ // Process
export const onSigint = () => {
  process.stdout.write('\u001B[?25h');
};

/* c8 ignore next */ // Process
process.once('SIGINT', onSigint);

---

I'll try to investigate some way of being able to test specific scenarios with process.

[wellwelwel]

👥 For community

Note

If you don't agree with any of these not covered topics, please feel free to comment. I'm totally open to discussions and opinions.

Different behaviors due to platform versions

• Compatibility with c8 starts with Node.js v18.
• Compatibility with monocart-coverage-reports starts with Node.js v18.

Different behaviors due to Deno and Bun platforms

• At the moment, there is no way to generate the coverage report for them using c8 or monocart-coverage-reports.

process-based

E.g., process.exit, process.once, etc.

• I intend to do more research.

Different behaviors due to OS

Windows, specifically.

• I'm planning to remove this one, since we can use Windows matrix to ensure this.

---

The choice not to consider these topics in the coverage comes from the fact that it isn't possible, not that they are ignored. For example, even if exhaustive tests are created for Node.js v8, there is no way to generate the coverage report for these tests.

Similarly, there are specific tests for Bun and Deno that don't generate coverage reports.

In disagreements, this issue can be reopened at any time 🤝

---

Related

• ci: improve project coverage rate transparency #585

[wellwelwel]

[...] The high coverage rate was one of the aspects why I decided to migrate to Poku
[...] but now for me it feels misleading

Less formal, I don't want anyone to feel that way when using Poku, which is why I'm taking it so seriously 💙

[mrazauskas]

I appreciate the effort. Thank you.

[wellwelwel]

Converting this issue to a discussion in favor of #612.