Walkthrough

Migrates CI coverage tooling from nyc to c8: updates .c8rc.json, replaces npx nyc with npx c8 in Linux and Windows workflows, adds c8 and removes nyc from devDependencies, and adjusts .gitignore entries related to coverage and node_modules.

Changes

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant CI as CI runner
  participant Shell as Shell
  participant Tests as Test script

  Note over CI,Shell: Previous flow (nyc)
  CI->>Shell: run "npx nyc npm run test:ci"
  Shell->>Tests: npm run test:ci (instrumented by nyc)
  Tests-->>Shell: test results + nyc output
  Shell-->>CI: exit status + coverage artifacts

  Note over CI,Shell `#DDEEFF`: Updated flow (c8)
  CI->>Shell: run "npx c8 npm run test:ci"
  Shell->>Tests: npm run test:ci (instrumented by c8)
  Tests-->>Shell: test results + c8 output
  Shell-->>CI: exit status + coverage artifacts

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Verify .c8rc.json keys/values (reporter array, exclude-after-remap) match c8 expectations.
• Confirm all npx nyc occurrences were replaced and CI artifact/log paths still align.
• Ensure removal of .nyc_output/, coverage/, and node_modules/ from .gitignore is intentional.

Suggested reviewers

• sgarner
• naorpeled
• alumni
• mguida22
• gioboa

Poem

🐰 I hopped from nyc to c8 tonight,

swapping tools by soft moonlight.
Workflows tuned and dev deps swapped,
configs trimmed, the pipelines hopped.
A carrot cheer — the tests all bright!

Pre-merge checks and finishing touches

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

typeorm-sql-js-example

npm i https://pkg.pr.new/typeorm/typeorm@11759

commit: 6a1c5d3

We were going to switch to Vitest in the future, is this change necessary?

We were going to switch to Vitest in the future, is this change necessary?

TLDR: not absolutely necessary but should bring immediate CI speed improvement

Vitest is used to use c8 (no using native v8 coverage), but seems that migration will take a while. And this should bring immediate benefit of speeding up CI. It's a drop in replacement so not a lot of work to do it and not much to remove it in the future...

I fired it up to do a quick comparison (c8 vs nyc)

• MongoDB on linux: 49s vs 59s
• MSSQL on linux: 4m 3s vs 4m 18s
• PostgreSQL on linux: 2m 3s vs 2m 17s
• Sqlite on windows: 2m 24s vs 2m 47s

Two random runs is not enough to calculate fair speed improvement, but it shaves off 10-20s, which is not bad given how little effort it is to do it.

And migration to vitest might take a while given how little progress has been made so far: master...next

I wonder why the coverage CI/CD part is skipped

I wonder why the coverage CI/CD part is skipped

It's not. It just wait for all tests to fininsh:

https://github.com/typeorm/typeorm/blob/master/.github/workflows/commit-validation.yml#L67

Its a good question though why oracle on linux started to fail now...

Can you please add html reporter?

Looks like lcov already provides html report.

After testing it generates 0 coverage, which is no good. Can you please investigate what's wrong?

At least it generate 0 coverage on my machine.

Switching back to nyc shows the correct coverage on my local machine.

Looks like lcov already provides html report.

After testing it generates 0 coverage, which is no good. Can you please investigate what's wrong?

Yes, I am investigating. But I can't get the tests pass on my local machine :/ Testing with mysql-9...

coverage: 80.769% (+4.3%) from 76.433%
when pulling a1409ba on pkuczynski:ci/c8
into 51fbcf4 on typeorm:master.

This PR would be nice to have if the reporting gets fixed.

Currently, nyc brings in baseline-browser-mapping which logs a warning that breaks our tests. If we get c8 working, we wouldn't need to figure out a way to fix that test :)

After adding --exclude-after-remap parameter to c8 like this: npx c8 --exclude-after-remap npm run test:ci it started to show the coverage.

I'm still wondering why the coverage is marked as skipped though all tests are passed/failed.

@OSA413 That's nice, thanks!

Do you happen to know where does the extra 4% coverage comes from?

BTW, Both nyc and c8 pass their options to test-exclude (which uses glob after that), so I doubt that anything related to the file globs should be changed. test-exclude automatically adds **/node_modules/** if you pass excludeNodeModules: true - which is correct, while node_modules without ** at the end does not do anything - so I doubt this change was responsible for the increased coverage.

Thank you for help @OSA413!

@alumni possibly it comes from how all: true + exclude-after-remap: true behaves differently than in nyc?

Here's the difference example (you can run SQLite tests locally and see it):

Old:

New:

Probably something related to TS -> JS (build) -> TS (Map back to source code) mapping.

I think that C8 can handle/track more things, that's why the coverage changed.