GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — test_runner: stringify AssertError expected and actual by MoLow · Pull Request #47088 · nodejs/node

Conversation

@MoLow

@MoLow MoLow commented Mar 14, 2023

Copy link
Copy Markdown
Member

Fixes: #47075

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@nodejs-github-bot

nodejs-github-bot commented Mar 14, 2023

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/test_runner
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot 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
MoLow commented Mar 14, 2023
View reviewed changes
Comment thread test/message/test_runner_output_spec_reporter.out Resolved

Uh oh!

There was an error while loading. Please reload this page.
aduh95
aduh95 reviewed Mar 14, 2023
View reviewed changes
Comment thread lib/internal/test_runner/reporter/tap.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
cjihrig
cjihrig reviewed Mar 14, 2023
View reviewed changes

@cjihrig cjihrig left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
MoLow reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread test/message/test_runner_output.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread test/message/test_runner_output.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread test/message/test_runner_output.out Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread lib/internal/test_runner/reporter/tap.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
@MoLow
MoLow force-pushed the stringify-assert-error-props branch 3 times, most recently from 6a60ab8 to b6d1159 Compare March 21, 2023 20:17
@MoLow
MoLow requested a review from cjihrig March 21, 2023 20:21
@MoLow
MoLow force-pushed the stringify-assert-error-props branch from b6d1159 to b540361 Compare March 28, 2023 19:20
cjihrig
cjihrig approved these changes Apr 2, 2023
View reviewed changes

@cjihrig cjihrig left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Sorry for the delay. LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread lib/internal/test_runner/reporter/tap.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread lib/internal/test_runner/yaml_to_js.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
@MoLow
test_runner: stringify AssertError expected and actual
8840601
@MoLow
MoLow force-pushed the stringify-assert-error-props branch from b540361 to 8840601 Compare April 3, 2023 16:22
@MoLow MoLow added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Apr 3, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Apr 3, 2023
@nodejs-github-bot

nodejs-github-bot commented Apr 3, 2023

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ljharb
ljharb approved these changes Apr 3, 2023
View reviewed changes
@nodejs-github-bot

nodejs-github-bot commented Apr 3, 2023

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Apr 3, 2023

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot mentioned this pull request Apr 4, 2023
CI Reliability 2023-04-04 nodejs/reliability#539
Open
29 tasks
@MoLow MoLow added the commit-queue PRs queued for automated landing through the Commit Queue. label Apr 4, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Apr 4, 2023
@nodejs-github-bot
nodejs-github-bot merged commit 102540a into nodejs:main Apr 4, 2023
@nodejs-github-bot

nodejs-github-bot commented Apr 4, 2023

Copy link
Copy Markdown
Collaborator

Landed in 102540a
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MoLow
MoLow deleted the stringify-assert-error-props branch April 4, 2023 07:34
@github-actions github-actions Bot mentioned this pull request Apr 5, 2023
CI Reliability 2023-04-05 nodejs/reliability#540
Open
26 tasks
RafaelGSS pushed a commit that referenced this pull request Apr 5, 2023
@MoLow @RafaelGSS
test_runner: stringify AssertError expected and actual
243546f 
PR-URL: #47088
Fixes: #47075
Reviewed-By: Colin Ihrig <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Apr 6, 2023
v19.9.0 proposal #47441
Merged
RafaelGSS pushed a commit that referenced this pull request Apr 6, 2023
@MoLow @RafaelGSS
test_runner: stringify AssertError expected and actual
fd1b825 
PR-URL: #47088
Fixes: #47075
Reviewed-By: Colin Ihrig <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 7, 2023
@MoLow @RafaelGSS
test_runner: stringify AssertError expected and actual
c8c7e03 
PR-URL: #47088
Fixes: #47075
Reviewed-By: Colin Ihrig <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 8, 2023
@MoLow @RafaelGSS
test_runner: stringify AssertError expected and actual
eb837ce 
PR-URL: #47088
Fixes: #47075
Reviewed-By: Colin Ihrig <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 13, 2023
@MoLow @RafaelGSS
test_runner: stringify AssertError expected and actual
1e9a500 
PR-URL: #47088
Fixes: #47075
Reviewed-By: Colin Ihrig <[email protected]>
danielleadams pushed a commit that referenced this pull request Jul 6, 2023
@MoLow @danielleadams
test_runner: stringify AssertError expected and actual
a65dd0b 
PR-URL: #47088
Fixes: #47075
Reviewed-By: Colin Ihrig <[email protected]>
MoLow added a commit to MoLow/node that referenced this pull request Jul 6, 2023
@MoLow
test_runner: stringify AssertError expected and actual
1d453e4 
PR-URL: nodejs#47088
Fixes: nodejs#47075
Reviewed-By: Colin Ihrig <[email protected]>
@danielleadams danielleadams mentioned this pull request Jul 10, 2023
v18.17.0 release proposal #48694
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

Uh oh!

There was an error while loading. Please reload this page.

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

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@MoLow @nodejs-github-bot @ljharb @cjihrig @aduh95
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.