Page MenuHomePhabricator

build: set different test thresholds for WIP components
Closed, ResolvedPublic

Description

In T313767, we set up a new directory in the codex package, /src/components-wip, to house work-in-progress components that are in an incomplete state. Since we did this in the name of making contributing to Codex easier, we should continue to lower the burden by reducing test thresholds for WIP components.


Acceptance criteria
  • Determine test thresholds
  • Set up test threshold specific to components-wip

Event Timeline

Right now we use a global coverage threshold, which computes a weighted average across all files and requires that that average be above a certain number (80% in our case). This means that you could add a new file that has very low coverage (even 0%), and as long as the rest of the code base has high enough coverage and the new uncovered file is small enough, it won't drag down the average enough to cause a coverage failure. You can see that phenomenon in action in our coverage report: branch coverage in Tabs.vue is just a little too low (about 78%), and coverage in useIntersectionObserver.ts is really low (20%), but the total coverage across all files (86.75%, shown at the top) is above 80%, so Jest thinks it's fine.

Jest gives us the option to define coverage thresholds on a per-file level, or on a directory level. Eventually I would like us to move to per-file thresholds (not meaning that we'd have different thresholds for each file, but that the threshold would be evaluated for each file separately, so no individual file would be allowed to fall below e.g. 80%). But for the time being, we could keep our global thresholds, and set a per-directory threshold for the components-wip directory. This would exclude the components-wip directory from the computation of the global threshold (essentially global means "everything else that doesn't have a more specific threshold"). I would suggest that we could set the threshold for components-wip to 0%, so that writing tests isn't required for an initial submission of a new component. Alternatively, we could set it to 1% for .vue files and 0% for everything else, to encourage people to add at least a basic snapshot test.

(For more information on how Jest coverage thresholds can be configured, see this documentation.)

+1 for eventually moving to per-file thresholds. I could be on board with a 1% threshold to encourage snapshot testing as you said, but even that can get frustrating if you encounter mysterious TypeScript issues (although we do have information on those issues in the docs). I particularly want to prevent people from having to wrestle with TypeScript when they just want to make a single contribution.

I'm in favor of a 1% coverage rule for these components. I think that contributors should be encouraged to provide some level of testing for a WIP submission, even if it's not very extensive.

Change 836940 had a related patch set uploaded (by Catrope; author: Catrope):

[design/codex@main] build: Exclude WIP code from test coverage threshold

https://gerrit.wikimedia.org/r/836940

I'm in favor of a 1% coverage rule for these components. I think that contributors should be encouraged to provide some level of testing for a WIP submission, even if it's not very extensive.

Unfortunately, this didn't work as I planned, because if there's no test file for a component at all, Jest doesn't discover that it exists and doesn't compute coverage for it. Maybe there's another Jest config setting that fixes that, but for now my patch just proposes a 0% threshold for the entire WIP directory.

Change 836940 merged by jenkins-bot:

[design/codex@main] build: Exclude WIP code from test coverage threshold

https://gerrit.wikimedia.org/r/836940

Closing this since there is no user-facing change to QA or sign off on.

Change 849191 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/core@master] Update Codex from v0.2.1 to v0.2.2

https://gerrit.wikimedia.org/r/849191

Change 849191 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.2.1 to v0.2.2

https://gerrit.wikimedia.org/r/849191