Skip to content

Commit

Permalink
Add a 'check-coverage' script so we can track towards completion
Browse files Browse the repository at this point in the history
We are targetting 100% test coverage for npm v7 GA.  Using a
coverage-map, we require that any of the newly created tests fully cover
the module that they are testing.  However, this has the side effect of
_always_ hitting 100% coverage when running 'npm test', even though not
all modules are being tested.

This adds a new 'load-all' test which, in 'check-coverage' mode, tells
nyc to include every file in the project.

This also removes the double-run of our tests in CI, where we run once
and then immediately run the same exact thing again for Coveralls, and
sends the 'check-coverage' output to Coveralls instead.

PR-URL: #1820
Credit: @isaacs
Close: #1820
Reviewed-by: @ruyadorno
  • Loading branch information
isaacs authored and ruyadorno committed Sep 22, 2020
1 parent a1d375f commit 1909ae0
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 4 deletions.
12 changes: 8 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,18 @@ jobs:
- name: Install dependencies
run: node . install

# Run the tests
# Run the tests, but not if we're just gonna do coveralls later anyway
- name: Run Tap tests
run: node . test -- -t600 -Rclassic -c
if: matrix.platform.os != 'ubuntu-latest' || matrix.node-version != '12.x'
run: node . test -- -t600 -Rbase -c
env:
DEPLOY_VERSION: testing

# Run coverage check
- name: Run coverage report
if: matrix.os == 'ubuntu-latest' && matrix.node-version == '12.x'
run: node . test -- -t600 -Rclassic -c
if: matrix.platform.os == 'ubuntu-latest' && matrix.node-version == '12.x'
# turn off --check-coverage until 100%, so CI failure is relevant
run: node . run check-coverage -- -t600 --no-check-coverage -Rbase -c
env:
DEPLOY_VERSION: testing
COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_OPTIONAL_TOKEN }}
Expand All @@ -55,5 +57,7 @@ jobs:
# if: matrix.os == 'ubuntu-latest'
# run: sudo PATH=$PATH $(which node) . test -- --coverage --timeout 600

# no need to check licenses everywhere, they don't change between versions
- name: Validate licenses
if: matrix.os == 'ubuntu-latest' && matrix.node-version == '12.x'
run: node . run licenses
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@
"preversion": "bash scripts/update-authors.sh && git add AUTHORS && git commit -m \"update AUTHORS\" || true",
"licenses": "licensee --production --errors-only",
"test": "tap",
"check-coverage": "tap",
"snap": "tap",
"test:nocleanup": "NO_TEST_CLEANUP=1 npm run test --",
"sudotest": "sudo npm run test --",
Expand Down
8 changes: 8 additions & 0 deletions test/coverage-map.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
'use strict'

const full = process.env.npm_lifecycle_event === 'check-coverage'
const coverageMap = (filename) => {
if (full && /load-all.js$/.test(filename)) {
const glob = require('glob')
const { resolve, relative } = require('path')
const dir = resolve(__dirname, '../lib')
return glob.sync(`${dir}/**/*.js`)
.map(f => relative(process.cwd(), f))
}
if (/^test\/(lib|bin)\//.test(filename)) {
return filename.replace(/^test\//, '')
}
Expand Down
30 changes: 30 additions & 0 deletions test/lib/load-all.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
const t = require('tap')
const glob = require('glob')
const { resolve } = require('path')

const full = process.env.npm_lifecycle_event === 'check-coverage'

if (!full) {
t.pass('nothing to do here, not checking for full coverage')
} else {
// some files do config.get() on load, so have to load npm first
const npm = require('../../lib/npm.js')
t.test('load npm first', t => npm.load(t.end))

t.test('load all the files', t => {
// just load all the files so we measure coverage for the missing tests
const dir = resolve(__dirname, '../../lib')
for (const f of glob.sync(`${dir}/**/*.js`)) {
require(f)
t.pass('loaded ' + f)
}
t.pass('loaded all files')
t.end()
})

t.test('call the error handle so we dont freak out', t => {
const errorHandler = require('../../lib/utils/error-handler.js')
errorHandler()
t.end()
})
}

0 comments on commit 1909ae0

Please sign in to comment.