chore: setup borp reporter for switch to node test #5720

dancastillo · 2024-09-29T21:34:00Z

Checklist

This PR install borp and adds test/test-reporter.mjs to run all tap and node:test test suites which will allow to incrementally migrate from tap to node:test.
This PR is insired from this comment #5683 (comment)

run npm run test and npm run benchmark
tests and/or benchmarks are included
commit message and code follows the Developer's Certification of Origin
and the Code of conduct

package.json

jsumners · 2024-10-02T13:29:17Z

test/test-reporter.mjs

+async function * reporter (source) {
+  const failed = new Set()
+  const diagnostics = new Set()
+  diagnostics.add('\n\n')


Let's leave formatting of output to the display section of the code and only display diagnostic messages when any exist (as we do with failed files).

test/test-reporter.mjs

package.json

dancastillo · 2024-10-08T13:54:04Z

Could this be an issue wiith borp on windows? Looks like macos and ubuntu run without issue
https://github.com/fastify/fastify/actions/runs/11236848075/job/31237887369?pr=5720

jsumners · 2024-10-08T14:59:45Z

It's due to --reporter=test/test-reporter.mjs and https://github.com/mcollina/borp/blob/339c6915bc2a782bea1df97e297bff6121c8f801/borp.js#L115-L129

For some reason, it's getting split into test and test-reporter.mjs. And then borp tries to load node_modules/test/ as a reporter.

jsumners · 2024-10-09T21:17:51Z

Unfortunately, I think we are going to need to extend borp to support an rc file in order to get around the various platforms's shell expansions. We should be able to write:

# .borp.yaml

reporters:
  - './test/test-reporter.mjs'

files:
  - 'test/**/*.test.js'
  - 'test/**/*.test.mjs'

jsumners · 2024-10-10T11:25:41Z

I have started the work for adding rcfile support to borp at mcollina/borp#105. Pretty much just needs some documentation. But my work day is about to start, so it'll have to wait (feel free to add to it with other PRs).

jsumners · 2024-10-11T17:36:05Z

@dancastillo bump borp to 0.18.0 and CI should pass.

jsumners · 2024-10-12T00:16:05Z

package.json

-    "coverage:ci": "tap --coverage-report=html --coverage-report=lcov --allow-incomplete-coverage",
-    "coverage:ci-check-coverage": "tap replay",
+    "coverage": "borp --reporter=./test/test-reporter.mjs --coverage --check-coverage --lines 100",
+    "coverage:ci": "nyc --coverage-report=html --coverage-report=lcov borp --reporter=./test/test-reporter.mjs --coverage --check-coverage",


We should be using c8 for coverage.

jsumners · 2024-10-12T00:16:32Z

package.json

-    "unit:report": "tap --coverage-report=html --coverage-report=cobertura | tee out.tap",
-    "citgm": "tap --jobs=1 --timeout=120"
+    "unit": "borp --reporter=./test/test-reporter.mjs",
+    "unit:report": "nyc --coverage-report=html --reporter=cobertura borp --reporter=./test/test-reporter.mjs --coverage --check-coverage",


nyc => c8

package.json

jsumners

Please add a .borp.yaml file with the following contents:

files:
  - 'test/**/*.test.js'
  - 'test/**/*.test.mjs'

We are missing at least one test suite without that configuration.

jsumners · 2024-10-12T12:26:43Z

package.json

-    "unit:report": "tap --coverage-report=html --coverage-report=cobertura | tee out.tap",
-    "citgm": "tap --jobs=1 --timeout=120"
+    "unit": "borp --reporter=./test/test-reporter.mjs --coverage --check-coverage",
+    "unit:report": "borp --reporter=./test/test-reporter.mjs --coverage > out.test-report",


This is not equivalent. It should be:

Suggested change

"unit:report": "borp --reporter=./test/test-reporter.mjs --coverage > out.test-report",

"unit:report": "c8 --reporter html borp --reporter=./test/test-reporter.mjs",

jsumners · 2024-10-12T12:31:43Z

package.json

-    "coverage:ci": "tap --coverage-report=html --coverage-report=lcov --allow-incomplete-coverage",
-    "coverage:ci-check-coverage": "tap replay",
+    "coverage": "borp --reporter=./test/test-reporter.mjs --coverage --check-coverage --lines 100",
+    "coverage:ci": "borp --reporter=./test/test-reporter.mjs --coverage --check-coverage",


It should be:

Suggested change

"coverage:ci": "borp --reporter=./test/test-reporter.mjs --coverage --check-coverage",

"coverage:ci": "c8 --reporter lcov --reporter html borp --reporter=./test/test-reporter.mjs",

jsumners · 2024-10-12T12:33:57Z

package.json

-    "coverage": "npm run unit -- --coverage-report=html",
-    "coverage:ci": "tap --coverage-report=html --coverage-report=lcov --allow-incomplete-coverage",
-    "coverage:ci-check-coverage": "tap replay",
+    "coverage": "borp --reporter=./test/test-reporter.mjs --coverage --check-coverage --lines 100",


Suggested change

"coverage": "borp --reporter=./test/test-reporter.mjs --coverage --check-coverage --lines 100",

"coverage": "c8 --reporter html --check-coverage --100 borp --reporter=./test/test-reporter.mjs",

borp.yaml

jsumners

🎉

mcollina

lgtm

mcollina · 2024-10-13T09:39:53Z

lib/logger.js

 * Repo: https://github.com/pinojs/pino-http
 * License: MIT (https://raw.githubusercontent.com/pinojs/pino-http/master/LICENSE)
 */
+/* c8 ignore stop */


I'm pretty sure these should not be needed

jsumners · 2024-10-13T11:56:25Z

I'm going to merge this so work can progress. @dancastillo please determine if the c8 directives Matteo called out are actually needed and follow-up with another PR if they can be removed.

github-actions · 2025-10-14T00:30:08Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

chore: setup borp reporter for switch to node test

2413572

metcoder95 reviewed Sep 30, 2024

View reviewed changes

package.json Outdated Show resolved Hide resolved

dancastillo marked this pull request as draft October 1, 2024 13:27

dancastillo mentioned this pull request Oct 1, 2024

Add @reporters/junit mcollina/borp#101

Closed

jsumners reviewed Oct 2, 2024

View reviewed changes

chore: update scripts and test-reporter

8c785e7

jsumners mentioned this pull request Oct 5, 2024

chore: Migrate to node:test #5714

Merged

3 tasks

dancastillo added 2 commits October 8, 2024 09:44

update scripts

445f139

Merge branch 'main' into borp-reporter-for-tap-and-nodetest

35b13e1

fix

c17b4dd

dancastillo mentioned this pull request Oct 9, 2024

fix: windows reporter reference mcollina/borp#104

Merged

bump borp to v0.18.0

4dece64

dancastillo marked this pull request as ready for review October 11, 2024 18:06

dancastillo requested review from jsumners and metcoder95 October 11, 2024 21:20

jsumners requested changes Oct 12, 2024

View reviewed changes

pr feedback

95eb898

dancastillo requested a review from jsumners October 12, 2024 02:12

jsumners requested changes Oct 12, 2024

View reviewed changes

pr feedback

1bd7d1f

jsumners reviewed Oct 12, 2024

View reviewed changes

borp.yaml Show resolved Hide resolved

fix file name to .borp.yaml

3cb7ce2

jsumners approved these changes Oct 12, 2024

View reviewed changes

metcoder95 approved these changes Oct 13, 2024

View reviewed changes

mcollina approved these changes Oct 13, 2024

View reviewed changes

jsumners mentioned this pull request Oct 13, 2024

refactor: migrate test framework in build module #5683

Closed

jsumners merged commit 025fa7a into fastify:main Oct 13, 2024

github-actions bot locked as resolved and limited conversation to collaborators Oct 14, 2025

	"unit:report": "borp --reporter=./test/test-reporter.mjs --coverage > out.test-report",
	"unit:report": "c8 --reporter html borp --reporter=./test/test-reporter.mjs",

	"coverage:ci": "borp --reporter=./test/test-reporter.mjs --coverage --check-coverage",
	"coverage:ci": "c8 --reporter lcov --reporter html borp --reporter=./test/test-reporter.mjs",

	"coverage": "borp --reporter=./test/test-reporter.mjs --coverage --check-coverage --lines 100",
	"coverage": "c8 --reporter html --check-coverage --100 borp --reporter=./test/test-reporter.mjs",

Uh oh!

chore: setup borp reporter for switch to node test #5720

chore: setup borp reporter for switch to node test #5720

Uh oh!

Conversation

dancastillo commented Sep 29, 2024

Checklist

Uh oh!

Uh oh!

jsumners Oct 2, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dancastillo commented Oct 8, 2024

Uh oh!

jsumners commented Oct 8, 2024

Uh oh!

jsumners commented Oct 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jsumners commented Oct 10, 2024

Uh oh!

jsumners commented Oct 11, 2024

Uh oh!

jsumners Oct 12, 2024

Choose a reason for hiding this comment

Uh oh!

jsumners Oct 12, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jsumners left a comment

Choose a reason for hiding this comment

Uh oh!

jsumners Oct 12, 2024

Choose a reason for hiding this comment

Uh oh!

jsumners Oct 12, 2024

Choose a reason for hiding this comment

Uh oh!

jsumners Oct 12, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jsumners left a comment

Choose a reason for hiding this comment

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

mcollina Oct 13, 2024

Choose a reason for hiding this comment

Uh oh!

jsumners commented Oct 13, 2024

Uh oh!

github-actions bot commented Oct 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jsumners commented Oct 9, 2024 •

edited

Loading