FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test_runner: stringify AssertError expected and actual by MoLow · Pull Request #47088 · nodejs/node · GitHub

/ node Public

test_runner: stringify AssertError expected and actual - #47088

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
MoLow:stringify-assert-error-props
Apr 4, 2023
ConversationCommits (1)ChecksFiles changed
Merged

test_runner: stringify AssertError expected and actual#47088
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
MoLow:stringify-assert-error-props

Conversation

MoLow commented Mar 14, 2023

Copy link
Copy Markdown
Member

Fixes: #47075

nodejs-github-bot commented Mar 14, 2023

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/test_runner

nodejs-github-bot added dont-land-on-v14.x needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Mar 14, 2023
MoLow commented Mar 14, 2023
View reviewed changes
Comment thread test/message/test_runner_output_spec_reporter.out Outdated
aduh95 reviewed Mar 14, 2023
View reviewed changes
Comment thread lib/internal/test_runner/reporter/tap.js Outdated
cjihrig reviewed Mar 14, 2023
View reviewed changes

cjihrig left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think this change might work, but I think it's incomplete without a context object. I also worry about how the stringify change might interact with large objects.

I'm not 100% if it would be valid YAML, but if it is, maybe actual and expected should add another level of indentation instead of just trying to stringify.

Comment thread test/message/test_runner_output.js Outdated
Comment thread test/message/test_runner_output.js Outdated
Comment thread test/message/test_runner_output.out Outdated
Comment thread lib/internal/test_runner/reporter/tap.js Outdated
MoLow force-pushed the stringify-assert-error-props branch 3 times, most recently from 6a60ab8 to b6d1159 Compare March 21, 2023 20:17
MoLow requested a review from cjihrig March 21, 2023 20:21
MoLow force-pushed the stringify-assert-error-props branch from b6d1159 to b540361 Compare March 28, 2023 19:20
cjihrig approved these changes Apr 2, 2023
View reviewed changes

cjihrig left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Sorry for the delay. LGTM

Comment thread lib/internal/test_runner/reporter/tap.js Outdated
Comment thread lib/internal/test_runner/yaml_to_js.js Outdated
MoLow force-pushed the stringify-assert-error-props branch from b540361 to 8840601 Compare April 3, 2023 16:22
MoLow added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 3, 2023
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 3, 2023

nodejs-github-bot commented Apr 3, 2023

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Apr 3, 2023

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Apr 3, 2023

Copy link
Copy Markdown
Collaborator

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

github-actions Bot mentioned this pull request Apr 4, 2023
Open
29 tasks
MoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 4, 2023
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 4, 2023
nodejs-github-bot merged commit 102540a into nodejs:main Apr 4, 2023

nodejs-github-bot commented Apr 4, 2023

Copy link
Copy Markdown
Collaborator

Landed in 102540a

MoLow deleted the stringify-assert-error-props branch April 4, 2023 07:34
github-actions Bot mentioned this pull request Apr 5, 2023
Open
26 tasks
RafaelGSS pushed a commit that referenced this pull request Apr 5, 2023
test_runner: stringify AssertError expected and actual
243546f 
PR-URL: #47088
Fixes: #47075
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
RafaelGSS mentioned this pull request Apr 6, 2023
Merged
RafaelGSS pushed a commit that referenced this pull request Apr 6, 2023
test_runner: stringify AssertError expected and actual
fd1b825 
PR-URL: #47088
Fixes: #47075
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Apr 7, 2023
test_runner: stringify AssertError expected and actual
c8c7e03 
PR-URL: #47088
Fixes: #47075
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Apr 8, 2023
test_runner: stringify AssertError expected and actual
eb837ce 
PR-URL: #47088
Fixes: #47075
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Apr 13, 2023
test_runner: stringify AssertError expected and actual
1e9a500 
PR-URL: #47088
Fixes: #47075
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
danielleadams pushed a commit that referenced this pull request Jul 6, 2023
test_runner: stringify AssertError expected and actual
a65dd0b 
PR-URL: #47088
Fixes: #47075
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MoLow added a commit to MoLow/node that referenced this pull request Jul 6, 2023
test_runner: stringify AssertError expected and actual
1d453e4 
PR-URL: nodejs#47088
Fixes: nodejs#47075
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
danielleadams mentioned this pull request Jul 10, 2023
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

aduh95 aduh95 left review comments
ljharb ljharb approved these changes
cjihrig cjihrig approved these changes

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

assert.deepEqual can generate invalid YAML output

5 participants

Back | FazBrowse Home | New Git URL