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

Coverage with c8 and tsx reporting as 100% since version 4.3.0 #433

Closed
4 of 6 tasks
OscarBarrett opened this issue Dec 8, 2023 · 11 comments
Closed
4 of 6 tasks

Coverage with c8 and tsx reporting as 100% since version 4.3.0 #433

OscarBarrett opened this issue Dec 8, 2023 · 11 comments
Labels
bug Something isn't working pr welcome released

Comments

@OscarBarrett
Copy link

Acknowledgements

  • I searched existing issues before opening this one to avoid duplicates
  • I understand this is not a place for seek help, but to report a bug
  • I understand that the bug must be proven first with a minimal reproduction
  • I will be polite, respectful, and considerate of people's time and effort

Minimal reproduction URL

https://github.com/OscarBarrett/tsx-coverage-issue

Version

v4.6.2

Node.js version

v20.10.0

Package manager

yarn

Operating system

Linux

Problem & Expected behavior

We use c8 and tsx together when running our test suites, and since tsx version 4.3.0 any file that is imported in our tests gets 100% coverage.
Locking to tsx 4.2.1, we get the expected coverage.

Assuming this is related to #405

Contributions

  • I plan to open a pull request for this issue
  • I plan to make a financial contribution to this project
@andreashuber69
Copy link

I have the same issue. FWIW, you can work around this by running tests on the built js output. If you have TypeScript generate .map files, then the CC output should look the same as before (i.e. reported on ts files rather than js files). Here's an example of the changes necessary (besides building with tsc):

andreashuber69/verify-coldcard-dice-seed@35b72b9

@jbergstroem

This comment has been minimized.

@privatenumber
Copy link
Owner

I haven't investigated this but I'm thinking this is as simple as removing this check so sourcemaps are always inlined:

if (hasNativeSourceMapSupport) {

There will need to be tests though. PR welcome if anyone wants to tackle.

@cenfun

This comment was marked as off-topic.

@privatenumber
Copy link
Owner

privatenumber commented Apr 4, 2024

Sorry, I'm going to hide your comment because it doesn't lead to a solution in tsx.

Seems like both versions of tsx in the reproduction doesn't work in the latest version of Node, but the reproduction still works fine with Node v20.10.0.

I'm curious what happened between tsx v4.2.1 and v4.6.2 that broke c8 in Node v20.10.0.

@cenfun
Copy link
Contributor

cenfun commented Apr 4, 2024

@privatenumber the generated sourcemap content could be different
could be caused by this

/**
	 * Improve performance by generating smaller source maps
	 * that doesn't include the original source code
	 *
	 * https://esbuild.github.io/api/#sources-content
	 */
	sourcesContent: false,

@privatenumber
Copy link
Owner

privatenumber commented Apr 4, 2024

@cenfun

You're right!

I tested removing sourcesContent (https://github.com/privatenumber/tsx/compare/esbuild-sourcesContent?expand=1) in the reproduction and the coverage is working again in Node 20.10.0 and v20.12.1 (latest LTS):

pnpm i 'privatenumber/tsx#npm/esbuild-sourcesContent'

(Here's how to make a installable branch, if you're interested)

I'll need to look into a way to add tests for this.

@privatenumber
Copy link
Owner

This has been completed in the development repo.

sverweij added a commit to sverweij/watskeburt that referenced this issue May 12, 2024
## Description

- replaces c8 with node 'native' --experimental-test-coverage

## Motivation and Context

c8 in our setup always returned 100% coverage for all things - which
might have to do with an issue in tsx (see [tsx
#433](privatenumber/tsx#433) ), but might just
as well be caused by something unexpectedly breaking in de nodejs / v8
w.r.t. loader coverage.

`--experimental-test-coverage` is not perfect either (i.c.w. tsx it's
unstable on node > 20.12.2, and line numbers are off), but at least the
coverage data we get is more realistic.

## How Has This Been Tested?

- [x] green ci

## Screenshots

```
> tsx --experimental-test-coverage --test-reporter ./tools/dot-with-summary.reporter.js --test src/*.spec.ts src/**/*.spec.ts

.................................................

49 passing (1.420 ms)

=============================== Coverage summary ===============================
Branches     : 97,89 % (93/95) NOK
Functions    : 100 % (38/38)
Lines        : 99,76 % (422/423)
================================================================================

Uncovered lines:
  /Users/sander/prg/js/watskeburt/src/git-primitives.ts:80

Uncovered branches:
  /Users/sander/prg/js/watskeburt/src/git-primitives.ts:79
  /Users/sander/prg/js/watskeburt/src/main.ts:22
```

## Types of changes

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] Documentation only change
- [ ] Refactor (non-breaking change which fixes an issue without
changing functionality)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [x] Chore
@privatenumber
Copy link
Owner

🎉 This issue has been resolved in v4.10.4

If you appreciate this project, please consider supporting this project by sponsoring ❤️ 🙏

antoinezanardi pushed a commit to antoinezanardi/werewolves-assistant-web-next that referenced this issue May 21, 2024
## [1.15.0](v1.14.0...v1.15.0) (2024-05-21)

### 🚀 Features

* **game:** options hub ([#407](#407)) ([16dc0ba](16dc0ba))

### 🐛 Bug Fixes

* **accessibility:** nav and list issues ([#417](#417)) ([c2da1f1](c2da1f1))

### 🧹 Chore

* **deps:** bump @nuxt/test-utils from 3.12.1 to 3.13.0 in the nuxt group ([#409](#409)) ([2ce6348](2ce6348))
* **deps:** bump @nuxt/test-utils from 3.13.0 to 3.13.1 in the nuxt group ([#413](#413)) ([89ff57e](89ff57e)), closes [#849](#849) [#851](#851) [#849](#849) [#851](#851) [#845](#845)
* **deps:** bump happy-dom from 14.10.1 to 14.10.2 ([#405](#405)) ([1c89b29](1c89b29))
* **deps:** bump happy-dom from 14.10.2 to 14.11.0 ([#408](#408)) ([12258c3](12258c3))
* **deps:** bump sass from 1.77.1 to 1.77.2 ([#411](#411)) ([9c047d3](9c047d3))
* **deps:** bump the eslint group with 2 updates ([#414](#414)) ([761b21e](761b21e))
* **deps:** bump the eslint group with 3 updates ([#403](#403)) ([20bc95e](20bc95e))
* **deps:** bump the eslint group with 3 updates ([#416](#416)) ([37a1346](37a1346))
* **deps:** bump tsx from 4.10.1 to 4.10.2 ([#404](#404)) ([616d779](616d779))
* **deps:** bump tsx from 4.10.2 to 4.10.4 ([#412](#412)) ([c6fc544](c6fc544)), closes [#18](#18) [privatenumber/tsx#433](privatenumber/tsx#433) [#18](#18) [#555](#555)
* **deps:** bump tsx from 4.10.4 to 4.10.5 ([#415](#415)) ([5c37023](5c37023))
@wellwelwel
Copy link

wellwelwel commented May 30, 2024

🎉 This issue has been resolved in v4.10.4

An observation that I find interesting to relate is that after v4.10.4, the coverage of a project dropped by almost 2%.
Looking at the differences between tsx versions, I noticed that it was mainly the imports of types.

To fix it, I just used c8's recommendation:

/* c8 ignore next */
import type { SomeType } from '../some-type.js';
  • I don't see this as an issue, so I preferred to add it to the context of this issue.

@privatenumber, thanks for this project and the fix 💙

@rozzilla
Copy link

Hi @privatenumber 👋🏼 😊
I also have a similar issue. I opened it here.

Repository owner locked and limited conversation to collaborators May 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working pr welcome released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants