Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

restore code coverage #4841

Merged
merged 16 commits into from
Jul 12, 2023
8 changes: 8 additions & 0 deletions .c8rc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"exclude": [
"lib/configValidator.js",
"lib/error-serializer.js",
"build/build-error-serializer.js",
"test/*"
]
}
2 changes: 0 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,6 @@ jobs:
- name: Run tests
run: |
npm run unit
env:
NODE_OPTIONS: no-network-family-autoselection

test-typescript:
needs:
Expand Down
2 changes: 0 additions & 2 deletions .github/workflows/coverage-nix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ jobs:
- name: Generate coverage report
run: |
npm run coverage:ci
env:
NODE_OPTIONS: no-network-family-autoselection

- uses: actions/upload-artifact@v3
if: ${{ success() }}
Expand Down
2 changes: 0 additions & 2 deletions .github/workflows/coverage-win.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ jobs:
- name: Generate coverage report
run: |
npm run coverage:ci
env:
NODE_OPTIONS: no-network-family-autoselection

- uses: actions/upload-artifact@v3
if: ${{ success() }}
Expand Down
4 changes: 2 additions & 2 deletions .taprc
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
ts: false
jsx: false
flow: false
check-coverage: true
coverage: true
check-coverage: false
mcollina marked this conversation as resolved.
Show resolved Hide resolved
coverage: false
node-arg: --allow-natives-syntax

files:
Expand Down
1 change: 1 addition & 0 deletions fastify.js
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,7 @@ function fastify (options) {
// https://github.com/nodejs/node/issues/48604
if (!options.serverFactory || fastify[kState].listening) {
instance.server.close(function (err) {
/* c8 ignore next 6 */
if (err && err.code !== 'ERR_SERVER_NOT_RUNNING') {
done(null)
} else {
Expand Down
7 changes: 3 additions & 4 deletions lib/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ function defaultResolveServerListeningText (address) {
function createServer (options, httpHandler) {
const server = getServerInstance(options, httpHandler)

return { server, listen }

// `this` is the Fastify object
function listen (listenOptions, ...args) {
let cb = args.slice(-1).pop()
Expand Down Expand Up @@ -99,6 +97,8 @@ function createServer (options, httpHandler) {

this.ready(listenCallback.call(this, server, listenOptions))
}

return { server, listen }
}

function multipleBindings (mainServer, httpHandler, serverOpts, listenOptions, onListen) {
Expand Down Expand Up @@ -138,17 +138,16 @@ function multipleBindings (mainServer, httpHandler, serverOpts, listenOptions, o
// pace through the `onClose` hook.
// It also allows them to handle possible connections already
// attached to them if any.
/* c8 ignore next 20 */
this.onClose((instance, done) => {
if (instance[kState].listening) {
// No new TCP connections are accepted
// We swallow any error from the secondary
// server
secondaryServer.close(() => done())
/* istanbul ignore next: Cannot test this without Node.js core support */
if (serverOpts.forceCloseConnections === 'idle') {
// Not needed in Node 19
secondaryServer.closeIdleConnections()
/* istanbul ignore next: Cannot test this without Node.js core support */
} else if (typeof secondaryServer.closeAllConnections === 'function' && serverOpts.forceCloseConnections) {
secondaryServer.closeAllConnections()
}
Expand Down
13 changes: 7 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,23 @@
"benchmark": "npx concurrently -k -s first \"node ./examples/benchmark/simple.js\" \"npx autocannon -c 100 -d 30 -p 10 localhost:3000/\"",
"benchmark:parser": "npx concurrently -k -s first \"node ./examples/benchmark/parser.js\" \"npx autocannon -c 100 -d 30 -p 10 -i ./examples/benchmark/body.json -H \"content-type:application/jsoff\" -m POST localhost:3000/\"",
"build:validation": "node build/build-error-serializer.js && node build/build-validation.js",
"coverage": "cross-env NODE_OPTIONS=no-network-family-autoselection npm run unit -- --cov --coverage-report=html",
"coverage:ci": "npm run unit -- --cov --coverage-report=html --no-browser --no-check-coverage",
"coverage:ci-check-coverage": "nyc check-coverage --branches 100 --functions 100 --lines 100 --statements 100",
"coverage": "npm run unit -- --coverage-report=html",
"coverage:ci": "npm run unit -- --coverage-report=html --no-browser --no-check-coverage",
mcollina marked this conversation as resolved.
Show resolved Hide resolved
"coverage:ci-check-coverage": "c8 check-coverage --branches 100 --functions 100 --lines 100 --statements 100",
Eomm marked this conversation as resolved.
Show resolved Hide resolved
"license-checker": "license-checker --production --onlyAllow=\"MIT;ISC;BSD-3-Clause;BSD-2-Clause\"",
"lint": "npm run lint:standard && npm run lint:typescript && npm run lint:markdown",
"lint:fix": "standard --fix",
"lint:markdown": "markdownlint-cli2",
"lint:standard": "standard | snazzy",
"lint:typescript": "eslint -c types/.eslintrc.json types/**/*.d.ts test/types/**/*.test-d.ts",
"prepublishOnly": "cross-env PREPUBLISH=true tap --no-check-coverage test/build/**.test.js && npm run test:validator:integrity",
"test": "npm run lint && cross-env NODE_OPTIONS=no-network-family-autoselection npm run unit && npm run test:typescript",
"test": "npm run lint && npm run unit && npm run test:typescript",
mcollina marked this conversation as resolved.
Show resolved Hide resolved
"test:ci": "npm run unit -- --cov --coverage-report=lcovonly && npm run test:typescript",
"test:report": "npm run lint && npm run unit:report && npm run test:typescript",
"test:validator:integrity": "npm run build:validation && git diff --quiet --ignore-all-space --ignore-blank-lines --ignore-cr-at-eol lib/error-serializer.js && git diff --quiet --ignore-all-space --ignore-blank-lines --ignore-cr-at-eol lib/configValidator.js",
"test:typescript": "tsc test/types/import.ts && tsd",
"test:watch": "npm run unit -- -w --no-coverage-report -R terse",
"unit": "tap",
"unit": "c8 tap",
"unit:junit": "tap-mocha-reporter xunit < out.tap > test/junit-testresults.xml",
"unit:report": "tap --cov --coverage-report=html --coverage-report=cobertura | tee out.tap"
},
Expand Down Expand Up @@ -145,6 +145,7 @@
"ajv-i18n": "^4.2.0",
"ajv-merge-patch": "^5.0.1",
"branch-comparer": "^1.1.0",
"c8": "^8.0.0",
"cross-env": "^7.0.3",
"eslint": "^8.39.0",
"eslint-config-standard": "^17.0.0",
Expand All @@ -169,7 +170,7 @@
"send": "^0.18.0",
"simple-get": "^4.0.1",
"snazzy": "^9.0.0",
"split2": "^4.2.0",
"split3": "^4.2.0",
Uzlopak marked this conversation as resolved.
Show resolved Hide resolved
"standard": "^17.0.0",
"tap": "^16.3.4",
"tsd": "^0.28.1",
Expand Down
4 changes: 4 additions & 0 deletions test/500s.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ test('default 500', t => {
t.plan(4)

const fastify = Fastify()
t.teardown(fastify.close.bind(fastify))

fastify.get('/', function (req, reply) {
reply.send(new Error('kaboom'))
Expand All @@ -33,6 +34,7 @@ test('custom 500', t => {
t.plan(6)

const fastify = Fastify()
t.teardown(fastify.close.bind(fastify))

fastify.get('/', function (req, reply) {
reply.send(new Error('kaboom'))
Expand Down Expand Up @@ -62,6 +64,7 @@ test('encapsulated 500', t => {
t.plan(10)

const fastify = Fastify()
t.teardown(fastify.close.bind(fastify))

fastify.get('/', function (req, reply) {
reply.send(new Error('kaboom'))
Expand Down Expand Up @@ -113,6 +116,7 @@ test('custom 500 with hooks', t => {
t.plan(7)

const fastify = Fastify()
t.teardown(fastify.close.bind(fastify))

fastify.get('/', function (req, reply) {
reply.send(new Error('kaboom'))
Expand Down