Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add stdout check to test-esm-cjs-main #25073

Closed
wants to merge 2 commits into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Dec 16, 2018

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Dec 16, 2018
@Trott
Copy link
Member Author

Trott commented Dec 16, 2018

Copy link
Member

@richardlau richardlau left a comment

Choose a reason for hiding this comment

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

LGTM if we rename the parameter.

test/es-module/test-esm-cjs-main.js Outdated Show resolved Hide resolved
@Trott
Copy link
Member Author

Trott commented Dec 16, 2018

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 16, 2018
@Trott
Copy link
Member Author

Trott commented Dec 16, 2018

Note to whoever lands this (which will probably be me): The commit message needs to be edited as it's checking signal rather than stdout.

@Trott
Copy link
Member Author

Trott commented Dec 16, 2018

Only red in the CI seems to be because the cleanup detected that @gireeshpunathil was testing something on CI:

21:45:21 ps awwx | grep Release/node | grep -v grep | cat
21:45:21  32506076  pts/2 A     0:00 /home/iojs/gireesh/exitrace/node/out/Release/node -e console.log('foobar'); 
21:45:21 gmake[1]: *** [test-ci] Error 1

If we see this happening a lot, we'll mark the node offline in Jenkins while they're doing their work (which is something we should do anyway, but we'd need some pretty responsive/on-duty Build folks to oversee it because right now we have them on one AIX node and @gabrielschulhof working on the other, so we'd have no AIX nodes left online potentially).

TL;DR Red CI here is unrelated and we'll Resume Build anyway.

@Trott
Copy link
Member Author

Trott commented Dec 16, 2018

Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/19585/ ✔️

@Trott
Copy link
Member Author

Trott commented Dec 18, 2018

Landed in dbdfc5d

@Trott Trott closed this Dec 18, 2018
Trott added a commit to Trott/io.js that referenced this pull request Dec 18, 2018
PR-URL: nodejs#25073
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 25, 2018
PR-URL: #25073
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Dec 25, 2018
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
PR-URL: nodejs#25073
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this pull request Apr 17, 2019
PR-URL: #25073
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this pull request Apr 28, 2019
PR-URL: #25073
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BethGriggs BethGriggs mentioned this pull request May 1, 2019
MylesBorins pushed a commit that referenced this pull request May 16, 2019
PR-URL: #25073
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants