Skip to content

test_runner: calculate executed lines using source map - #53315

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
MoLow:fix-source-map-original-lines
Jun 5, 2024
Merged

test_runner: calculate executed lines using source map#53315
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
MoLow:fix-source-map-original-lines

Conversation

@MoLow

@MoLowMoLow commented Jun 3, 2024

Copy link
Copy Markdown
Member

Fixes#52775 (comment)
Supersedes #53153

before this fix, the lines that were actually executed were calculated using the source file on the disk - this is not the case when using ES loaders/transpilers.
since we don't need the actual code run we create fake lines with a dummy char, just to map the dummy line to its original source

@MoLow
MoLow requested review from benjamingr and cjihrigJune 3, 2024 22:15
@nodejs-github-bot

nodejs-github-bot commented Jun 3, 2024

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Jun 3, 2024
@MoLowMoLow mentioned this pull request Jun 3, 2024
Closed
@khaosdoctor

khaosdoctor commented Jun 3, 2024

Copy link
Copy Markdown
Member

Amazing work! Thanks a lot for this!

cjihrig
cjihrig reviewed Jun 3, 2024
View reviewed changes
Comment threadtest/fixtures/test-runner/coverage-loader/hooks.mjs Outdated
Comment threadtest/fixtures/test-runner/coverage-loader/sum.test.ts Outdated
Comment threadtest/fixtures/test-runner/coverage-loader/sum.test.ts Outdated
Comment threadlib/internal/test_runner/coverage.js Outdated
Comment threadlib/internal/test_runner/coverage.js Outdated
@MoLow
MoLow requested a review from cjihrigJune 4, 2024 05:19
@MoLowMoLow added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Jun 4, 2024
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 4, 2024
@nodejs-github-bot

nodejs-github-bot commented Jun 4, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/59653/

@MoLowMoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 4, 2024
@MoLowMoLow added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Jun 5, 2024
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 5, 2024
@nodejs-github-bot
nodejs-github-bot merged commit 419172f into nodejs:mainJun 5, 2024
@nodejs-github-bot

nodejs-github-bot commented Jun 5, 2024

Copy link
Copy Markdown
Collaborator

Landed in 419172f

@MoLow
MoLow deleted the fix-source-map-original-lines branch June 6, 2024 05:34
@privatenumberprivatenumber mentioned this pull request Jun 6, 2024
Open
6 tasks
RafaelGSS pushed a commit that referenced this pull request Jun 7, 2024
@MoLow@RafaelGSS
test_runner: calculate executed lines using source map
2d1ecbf
PR-URL: #53315
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
@RafaelGSSRafaelGSS mentioned this pull request Jun 7, 2024
Merged
@SuperchupuDevSuperchupuDev mentioned this pull request Jun 13, 2024
Closed
6 tasks
soophoo pushed a commit to soophoo/node that referenced this pull request Jun 20, 2024
@MoLow@soophoo
test_runner: calculate executed lines using source map
bd81e22
PR-URL: nodejs#53315
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
@khaosdoctorkhaosdoctor mentioned this pull request Jun 25, 2024
Closed
marco-ippolito pushed a commit that referenced this pull request Jul 19, 2024
@MoLow@marco-ippolito
test_runner: calculate executed lines using source map
0381817
PR-URL: #53315
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
@marco-ippolitomarco-ippolito mentioned this pull request Jul 19, 2024
Merged
@yume-chanyume-chan mentioned this pull request Aug 16, 2024
Closed
@metcoder95metcoder95 mentioned this pull request Jun 25, 2025
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benjamingrbenjamingrbenjamingr approved these changes
@cjihrigcjihrigcjihrig approved these changes
@atlowChemiatlowChemiatlowChemi approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.needs-ciPRs that need a full CI run.test_runnerIssues and PRs related to the test runner subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Experimental Test Coverage throws "cannot read properties of undefined (reading: line)"

6 participants

@MoLow@nodejs-github-bot@khaosdoctor@benjamingr@cjihrig@atlowChemi