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: fix broken assertion by cjihrig · Pull Request #10840 · nodejs/node

Conversation

@cjihrig

@cjihrig cjihrig commented Jan 16, 2017

Copy link
Copy Markdown
Contributor

This commit fixes a broken assertion in test-cli-eval.js. b9adbf2 introduced the break, and was landed even though CI showed that the test failed.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

test

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@nodejs-github-bot nodejs-github-bot added test Issues and PRs related to Node.js core tests and test infrastructure. dont-land-on-v7.x labels Jan 16, 2017
@cjihrig cjihrig mentioned this pull request Jan 16, 2017
test: assert unused vars in test-cli-eval.js #10759
Closed
3 tasks done
@cjihrig

cjihrig commented Jan 16, 2017 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

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

Trying again: https://ci.nodejs.org/job/node-test-pull-request/5899/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
italoacasas
italoacasas approved these changes Jan 16, 2017
View reviewed changes
@italoacasas italoacasas removed the dont-land-on-v7.x label Jan 16, 2017
@mscdex mscdex added the process Issues and PRs related to the process subsystem. label Jan 16, 2017
@cjihrig
test: fix broken assertion
013da32 
This commit fixes a broken assertion in test-cli-eval.js.
@cjihrig
cjihrig force-pushed the broken-test branch from eda39c0 to 013da32 Compare January 16, 2017 22:39
@nodejs-github-bot nodejs-github-bot added the dont-land-on-v7.x label Jan 16, 2017
jasnell
jasnell approved these changes Jan 16, 2017
View reviewed changes
@cjihrig

cjihrig commented Jan 16, 2017

Copy link
Copy Markdown
Contributor Author

CI is green. May want to fast track this to unbreak the CI.
All reactions

Sorry, something went wrong.

Uh oh!

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

italoacasas commented Jan 16, 2017

Copy link
Copy Markdown

+1 for fast tracking this
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
addaleax
addaleax approved these changes Jan 17, 2017
View reviewed changes

@addaleax addaleax left a comment

Copy link
Copy Markdown
Member

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.

LGTM, landing

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@addaleax

addaleax commented Jan 17, 2017

Copy link
Copy Markdown
Member

Landed in 38ae95b
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@addaleax addaleax closed this Jan 17, 2017
addaleax pushed a commit that referenced this pull request Jan 17, 2017
@cjihrig @addaleax
test: fix broken assertion
38ae95b 
This commit fixes a broken assertion in test-cli-eval.js.

PR-URL: #10840
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Italo A. Casas <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
@jasnell

jasnell commented Jan 17, 2017

Copy link
Copy Markdown
Member

btw, my apologies on this. I'd completely missed the breakage there somehow.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@cjihrig
cjihrig deleted the broken-test branch January 17, 2017 02:13
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 18, 2017
@cjihrig Italo A. Casas
test: fix broken assertion
46575e2 
This commit fixes a broken assertion in test-cli-eval.js.

PR-URL: nodejs#10840
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Italo A. Casas <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
@targos targos removed the dont-land-on-v7.x label Jan 28, 2017
targos pushed a commit that referenced this pull request Jan 28, 2017
@cjihrig @targos
test: fix broken assertion
63fa05d 
This commit fixes a broken assertion in test-cli-eval.js.

PR-URL: #10840
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Italo A. Casas <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
@italoacasas italoacasas mentioned this pull request Jan 29, 2017
v7.5.0 proposal #11062
Merged
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 30, 2017
@cjihrig Italo A. Casas
test: fix broken assertion
32da59a 
This commit fixes a broken assertion in test-cli-eval.js.

PR-URL: nodejs#10840
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Italo A. Casas <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 30, 2017
@cjihrig Italo A. Casas
test: fix broken assertion
afd80d4 
This commit fixes a broken assertion in test-cli-eval.js.

PR-URL: nodejs#10840
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Italo A. Casas <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
@MylesBorins MylesBorins added dont-land-on-v4.x labels Mar 8, 2017
@MylesBorins MylesBorins added land-on-v6.x and removed dont-land-on-v6.x labels Apr 18, 2017
MylesBorins pushed a commit that referenced this pull request Apr 18, 2017
@cjihrig @MylesBorins
test: fix broken assertion
1a05f4a 
This commit fixes a broken assertion in test-cli-eval.js.

PR-URL: #10840
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Italo A. Casas <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this pull request Apr 19, 2017
@cjihrig @MylesBorins
test: fix broken assertion
b6c30e1 
This commit fixes a broken assertion in test-cli-eval.js.

PR-URL: #10840
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Italo A. Casas <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Apr 19, 2017
v6.10.3 proposal #12498
Merged
andrew749 pushed a commit to michielbaird/node that referenced this pull request Jul 19, 2017
@cjihrig @andrew749
test: fix broken assertion
fb59cda 
This commit fixes a broken assertion in test-cli-eval.js.

PR-URL: nodejs/node#10840
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Italo A. Casas <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes
@addaleax addaleax addaleax approved these changes
+1 more reviewer
@italoacasas italoacasas italoacasas approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

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

Assignees

No one assigned

Labels

process Issues and PRs related to the process subsystem. test Issues and PRs related to Node.js core tests and test infrastructure.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

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

8 participants

@cjihrig @italoacasas @addaleax @jasnell @mscdex @MylesBorins @targos @nodejs-github-bot
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.