Skip to content

Gracefully exit with code 1 when build failed#160

Merged
jaredpalmer merged 1 commit intojaredpalmer:masterfrom
jooohn:master
Jul 5, 2019
Merged

Gracefully exit with code 1 when build failed#160
jaredpalmer merged 1 commit intojaredpalmer:masterfrom
jooohn:master

Conversation

@jooohn
Copy link
Copy Markdown
Contributor

@jooohn jooohn commented Jun 25, 2019

I found some problems with build command.

  1. It does exit with code 0 even when the build failed. I think it should be 1.
  2. When it fails, the error won't be caught and that causes UnhandledPromiseRejectionWarning.

@jooohn
Copy link
Copy Markdown
Contributor Author

jooohn commented Jun 25, 2019

Hmm, it's working on my local machine, and I tried this from the same container as Circle CI, but it works successfully...

$ yarn test --runInBand --no-cache --coverage
yarn run v1.16.0
$ jest --config ./test/jest.config.json --runInBand --no-cache --coverage

 RUNS  test/tests/tsdx-build.test.js
✓ Creating entry file 2.7 secs
✓ Building modules 1.3 secs
✓ Creating entry file 2.8 secs
✓ Building modules 1.1 secs
✓ Creating entry file 2.8 secs
(typescript) Error: /Users/j-tomioka/src/github.com/jooohn/tsdx/stage-build/src/index.ts(1,14): semantic error TS2322: Type '"123"' is not assignable to type 'number'.
Error: /Users/j-tomioka/src/github.com/jooohn/tsdx/stage-build/src/index.ts(1,14): semantic error TS2322: Type '"123"' is not assignable to type 'number'.
    at error (/Users/j-tomioka/src/github.com/jooohn/tsdx/node_modules/rollup/dist/rollup.js:9365:30)
    at Object.error (/Users/j-tomioka/src/github.com/jooohn/tsdx/node_modules/rollup/dist/rollup.js:15557:24)
    at Object.error (/Users/j-tomioka/src/github.com/jooohn/tsdx/node_modules/rollup/dist/rollup.js:15986:38)
    at RollupContext.error (/Users/j-tomioka/src/github.com/jooohn/tsdx/node_modules/rollup-plugin-typescript2/dist/rollup-plugin-typescript2.cjs.js:17187:30)
    at /Users/j-tomioka/src/github.com/jooohn/tsdx/node_modules/rollup-plugin-typescript2/dist/rollup-plugin-typescript2.cjs.js:24954:23
    at arrayEach (/Users/j-tomioka/src/github.com/jooohn/tsdx/node_modules/rollup-plugin-typescript2/dist/rollup-plugin-typescript2.cjs.js:532:11)
    at forEach (/Users/j-tomioka/src/github.com/jooohn/tsdx/node_modules/rollup-plugin-typescript2/dist/rollup-plugin-typescript2.cjs.js:9360:14)
    at printDiagnostics (/Users/j-tomioka/src/github.com/jooohn/tsdx/node_modules/rollup-plugin-typescript2/dist/rollup-plugin-typescript2.cjs.js:24927:5)
    at Object.transform (/Users/j-tomioka/src/github.com/jooohn/tsdx/node_modules/rollup-plugin-typescript2/dist/rollup-plugin-typescript2.cjs.js:26754:17)
 PASS  test/tests/tsdx-build.test.js (14.523s)tsdx/node_modules/rollup/dist/rollup.js:15679:25
 PASS  test/tests/utils-safePackageName.test.ts
----------|----------|----------|----------|----------|-------------------|
File      |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
----------|----------|----------|----------|----------|-------------------|
All files |      100 |      100 |      100 |      100 |                   |
 util.js  |      100 |      100 |      100 |      100 |                   |
----------|----------|----------|----------|----------|-------------------|

Test Suites: 2 passed, 2 total
Tests:       4 passed, 4 total
Snapshots:   0 total
Time:        17.796s
Ran all test suites.
✨  Done in 18.81s.

@Pruxis
Copy link
Copy Markdown

Pruxis commented Jun 28, 2019

Yes please! This is kind of blocking anyone from building a decent CI pipeline

@jaredpalmer
Copy link
Copy Markdown
Owner

Can we make the tests pass?

@jooohn
Copy link
Copy Markdown
Contributor Author

jooohn commented Jul 1, 2019

Hmm, I don't know why it failed earlier, but now passed...!

@Pruxis
Copy link
Copy Markdown

Pruxis commented Jul 1, 2019

Thanks @jooohn was going to do it this evening, thanks for giving me more free time ;)

@swyxio
Copy link
Copy Markdown
Collaborator

swyxio commented Jul 1, 2019

@jooohn what did you do to make it pass? i took a quick look and couldnt figure it out

@jooohn
Copy link
Copy Markdown
Contributor Author

jooohn commented Jul 1, 2019

basically just retried..

@jaredpalmer jaredpalmer merged commit c850b5c into jaredpalmer:master Jul 5, 2019
@agilgur5
Copy link
Copy Markdown
Collaborator

@allcontributors please add @jooohn for code, tests

@allcontributors
Copy link
Copy Markdown
Contributor

@agilgur5

I've put up a pull request to add @jooohn! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants