Skip to content
This repository was archived by the owner on Aug 11, 2022. It is now read-only.

run-script: keep exit code #15413

Merged
zkat merged 1 commit into from
Jan 25, 2017
Merged

run-script: keep exit code #15413

zkat merged 1 commit into from
Jan 25, 2017

Conversation

@kapals
Copy link
Contributor

@kapals kapals commented Jan 6, 2017

Keep non-zero exit codes same.

@kapals kapals closed this Jan 6, 2017
@kapals kapals reopened this Jan 6, 2017
@iarna
Copy link
Contributor

iarna commented Jan 9, 2017

This looks good, but the test you added is failing in CI (and when I try it out locally myself):

# Subtest: npm run-script transitional keep non-zero exit code
    ok 1 - ran run-script without parameters without crashing
    not ok 2 - got expected exit code
      ---
      found: 1
      wanted: 7
      compare: ===
      at:
        line: 351
        column: 7
        file: test/tap/run-script.js
      stack: |
        test/tap/run-script.js:351:7
        f (node_modules/once/once.js:25:25)
        ChildProcess.<anonymous> (test/common-tap.js:71:5)
        Pipe._onclose (net.js:486:12)
      source: |
        t.equal(code, 7, 'got expected exit code')
      ...
    ok 3 - should generate errors
    1..3
    # failed 1 of 3 tests

@kapals
Copy link
Contributor Author

kapals commented Jan 16, 2017

run-script within another run-script probably uses npm package from PATH, so I've removed the broken test-case.

@iarna iarna added this to the next milestone Jan 23, 2017
iarna
iarna approved these changes Jan 23, 2017
View reviewed changes
Copy link
Contributor

@iarna iarna left a comment

Choose a reason for hiding this comment

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

LGTM

@zkat zkat changed the base branch from latest to release-next January 25, 2017 14:10
@zkat zkat merged commit d75fc03 into npm:release-next Jan 25, 2017
@zkat zkat mentioned this pull request Feb 14, 2017
Closed
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@iarna iarna iarna approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

4.2.0

Development

Successfully merging this pull request may close these issues.

3 participants

@kapals @iarna @zkat