What Is the Default Coverage Threshold

[grant]

Describe a bug

I'm using this action with a repo setup that doesn't have a coverage threshold. For reference, our coverage is around 10-20%.

We're seeing St.❔ 🔴 for the status result. This is making engineers think something is wrong with PRs.

Questions:

• Can we remove this column for jest configs without a coverageThreshold? I wouldn't expect this action tell me if my code is above or below a threshold if I have not defined one in my config.
• What is the threshold? It could be in the alt text with St. ?.

I think the code is here:

jest-coverage-report-action/src/format/summary/formatCoverageSummary.ts

Line 23 in 952a059

getStatusOfPercents(currSummary.percentage, threshold),

Which defines a default threshold to 60?

jest-coverage-report-action/src/utils/getStatusOfPercents.ts

Line 5 in 952a059

export const getStatusOfPercents = (percentage: number, threshold = 60) => {

I would argue that 60 is not visible to the end user and shouldn't be set. Only the user or jest should be able to set a default config.

Expected behavior

Category	Percentage	Covered / Total
Statements	21.33% (-0% 🔻)	7162/33582
Branches	12.93% (-0% 🔻)	2412/18660
Functions	18.52%	1764/9524
Lines	21.32% (-0% 🔻)	6525/30599

Actual behavior

St.❔	Category	Percentage	Covered / Total
🔴	Statements	21.33% (-0% 🔻)	7162/33582
🔴	Branches	12.93% (-0% 🔻)	2412/18660
🔴	Functions	18.52%	1764/9524
🔴	Lines	21.32% (-0% 🔻)	6525/30599

Alternatives Considered

We will probably just make our threshold 0 to ensure PRs are 🟢 ok.

[grant]

It looks like this action assumes that the jest config is a json file (ours is not), and so it improperly gets the coverage threshold config value.

jest-coverage-report-action/src/utils/getNormalThreshold.ts

Line 10 in 952a059

const threshold = await tryGetJestThreshold(workingDirectory);

[grant]

Hi @ArtiomTr , do you know the answer to this?

Would you say that this default config is a bug? It would be nice if the default 0 so action comment aren't all red 🔴 symbolizing an error when there is none.

[ArtiomTr]

Hello @grant 👋,

Sorry for the late response. That is a quite complicated problem, although it seems simple at first glance. There are several issues, that occurred due to bad action architecture.

Long explanation

History

Initially, this action was created for serving a simple purpose: a better alternative to "just running tests". See, I was thinking that "just running tests" and measuring how "good" a pull request is, looking at the red/green bulb isn't a good way. I saw tools like Codecov, Coveralls, etc., but they seem too complicated for such a simple purpose.

So, I've started by doing a very simple thing: moving information that you see in a terminal when running jest with default reporters:

However, that was quite a lot of information to show in a single pull request, so I decided to reduce this information and keep only the "All" category. So, that's the reason why action produces these 4 categories in the output.

The text output is good, but that's not really human-readable. So, I decided to add these red/yellow/green icons, so the report will look nice & the person will immediately see some feedback.

However, using 60% as the threshold wasn't good for everyone. To deal with this issue, I decided to add an option, named threshold:

jest-coverage-report-action/action.yml

Lines 16 to 18 in 952a059

	threshold:
	required: false
	description: 'Coverage threshold. If total coverage is less than threshold, PR will be rejected'

Also, this option helped people automatically reject pull requests, when coverage drops dramatically.

And everything was cool, not taking into account the fact that this is absolutely wrong and even harmful. I was not familiar with all jest features, and I didn't know that there is already an option, named coverageThreshold. So, this option introduced a bad habit - when your CI can pass locally, but fail in PR. Also, this creates a small vendor lock-in issue: you're now forced to use this action as if you migrate to another action, it may handle coverage thresholds differently.

So, I've fixed coverage threshold checks, with a simple solution: because jest fails when coverageThreshold checks do not pass, I've just added graceful handling of these errors. And for the threshold option, I've just set up the coverageThreshold property correctly:

jest-coverage-report-action/src/utils/getNormalThreshold.ts

Lines 12 to 22 in 952a059

	// Should be removed in further versions
	if (isNil(threshold)) {
	return {
	global: {
	branches: thresholdFromOptions,
	functions: thresholdFromOptions,
	lines: thresholdFromOptions,
	statements: thresholdFromOptions,
	},
	};
	}

Now

So now, the action handles threshold checks almost correctly, but we have an issue with displaying the right colours for the bulbs (actually, the issue for this bug already exists #235). What is the problem with showing the right colour for each row?

It looks like this action assumes that the jest config is a json file (ours is not), and so it improperly gets the coverage threshold config value.

jest-coverage-report-action/src/utils/getNormalThreshold.ts

Line 10 in 952a059

const threshold = await tryGetJestThreshold(workingDirectory);

Actually, the action reads jest configuration properly, via the amazing c12 configuration loader.

The problem is that the project could have different threshold groups and even no threshold group for global. This means, that to show this correctly, action should generate the whole report differently. I'm sure that most users wouldn't be happy if I will break reports completely, in a minor version bump. I could do a major version bump, however, there are more things that I want to break 😄. Jest develops rapidly, and more and more features appear. For instance, here are some features that I want to add in the next major version:

1. Native support for jest shards Support for jest shards #286
2. Replace statement/branch/function annotations with line annotations maybe can add line comment #147
3. Speed up action by caching reports proposal: Cache base run results to speed up tests #136
4. Detailed coverage view (probably coming later, when GitHub Blocks will be stable)
5. And more...

I don't think that releasing 5-6 major bumps is a good solution, as people will be just frustrated by updating action every month. Also, this will mean that I would need to maintain 2-3 most recent major versions, which will be a total nightmare for me.

Can we just add another option?

I think that's the most commonly suggested solution to any problem. In most cases, adding a new option is a bad idea. There are several reasons, why it's bad:

1. Adding complexity to the action. Common, that is a small action, why it needs 13 options?
2. Maintainance hell. If we simplify, and say that those 13 options can have only 2 different values (on/off), then it's already 2^13=8192 different configurations. Of course, most of them won't conflict, but if someone finds an issue, I need to manually reproduce it, and fix it. Testing actions in GitHub isn't very easy: to perform an e2e test, I need to create the repository, pull request, and trigger action. Taking into account the fact, that there are countless options to create a repository on GitHub (public/private, personal/organisation, organisation privileges, cloud/self-hosting, ssh/https) it quickly becomes impossible to test all these cases.
3. DX degradation - it is always nice when action works right out of the box. Having 13 configuration options is clearly a problem because the user will need to configure something.

Conclusion

So, the real problem with this issue, is that I'm stuck in the loop: Try to fix bug -> Understand, that it will break things -> Decide to make make a major version to address this issue -> Find a ton of other bugs -> Try to fix bug -> and so on.

Of course, this is very funny: I can't change the colour of a single bulb 😄. I hope this explanation will make the reasoning behind the decision to "not address this issue" more clear.

And so, I propose a simple solution: I will create a separate branch, with the bulbs removed. Then, you could just use that branch, instead of the version. For instance, your action.yaml will look like this:

name: 'coverage'
on:
    pull_request:
        branches:
            - master
            - main
jobs:
    coverage:
        runs-on: ubuntu-latest
        steps:
            - uses: actions/checkout@v3
                                                        # ↓ branch name here 
            - uses: ArtiomTr/jest-coverage-report-action@without-bulbs 

That is not an ideal solution, as you won't receive any updates. But, this will fix your issue and will have 0 maintenance cost for me.

TLDR

1. This issue could not be easily fixed due to the structure of the coverageThreshold field in jest.config.js
2. I can fix that by creating an exclusive version, without those bulbs. It will be just a separate branch in a repository, and you could use it like this:

name: 'coverage'
on:
    pull_request:
        branches:
            - master
            - main
jobs:
    coverage:
        runs-on: ubuntu-latest
        steps:
            - uses: actions/checkout@v3
                                                        # ↓ branch name here 
            - uses: ArtiomTr/jest-coverage-report-action@without-bulbs 

Let me know if this solution will fit your needs.

[grant]

Thanks. I don't think we're going to use this Action for our use-case. For this issue, was just looking for an option to disable the column or use our config.

[mindrunner]

Facing similar issue. Would be nice to be able to disable bulbs as long as they show misleading information

[woochica]

@ArtiomTr thank you for the detailed explanation of this problem

[ab14bhardwaj]

@ArtiomTr Any updates on the above issue?

[StevenBoutcher]

@ArtiomTr I'd actually be grateful if you could make a separate repo without the bulbs. Is that offer still on the table?