Skip to content

Conversation

@hildjj
Copy link
Contributor

@hildjj hildjj commented Aug 17, 2025

  • chore(deps): updated all dependencies
  • fix(diagnostic): assign diagnostic to correct test

Fixes #207.

@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Aug 17, 2025
@hildjj
Copy link
Contributor Author

hildjj commented Aug 17, 2025

Also contains #208.

@hildjj hildjj changed the title fix 207 fix #207: assign diagnostic to correct test Aug 17, 2025
@dosubot dosubot bot added bug Something isn't working dependencies Pull requests that update a dependency file labels Aug 17, 2025

# Function to replace test duration with 0 and remove file paths
remove_variables() {
echo "$1" | sed -E 's/\"duration\": [0-9\.]+/\"duration\": 0/g' | sed -E 's/\/.*\/test\//test\//g'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: do not escape "." in a regexp class. The backslash becomes part of the class rather than escaping ".".


let diagnosticMessage = ''
const allTests = Object.create(null)
let currentTest
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a little bit of a hack to hold on to the last test, so that messages that come in without file, line, and column have somewhere to go.

parse-report.js Outdated
function lastTestInStack() {
return testStack.length ? testStack[testStack.length - 1] : null
function testId(data) {
return [data.file, data.line, data.column].join('\0')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Column is included so that tests that are run together in the same line are distinguishable.


function resetDiagnosticMessage() {
diagnosticMessage = ''
function appendDiagnosticMessage(data) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reordered these when I was going to use lastTestInStack. Can put them back in the previous order if this is confusing.

@simoneb
Copy link
Member

simoneb commented Aug 18, 2025

CI is failing. See my comment on the other PR.

@hildjj
Copy link
Contributor Author

hildjj commented Aug 18, 2025

Will rebase this on #208 once it lands.

@hildjj
Copy link
Contributor Author

hildjj commented Aug 20, 2025

This relies on #212 since the tests won't pass without it.

@simoneb
Copy link
Member

simoneb commented Aug 20, 2025

This relies on #212 since the tests won't pass without it.

#212 is merged, you can rebase. Thanks!

Ensure that calls to test.diagnostic are reflected
on the correct test.  Now captures previously-lost
top-level diagnostics as well.  Fixes nearform#207.
@hildjj
Copy link
Contributor Author

hildjj commented Aug 20, 2025

@simoneb rebased on master

Copy link
Member

@simoneb simoneb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks a lot for the contribution!

@simoneb simoneb merged commit 7ffa2fa into nearform:master Aug 20, 2025
3 checks passed
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Aug 20, 2025
@hildjj hildjj deleted the fix-207 branch August 20, 2025 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working dependencies Pull requests that update a dependency file lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

diagnostic not correct for nested tests

2 participants