[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
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.

Sign up for GitHub

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

Jump to bottom

Gracefully exit with code 1 when build failed #160

Merged
merged 1 commit into from
Jul 5, 2019
Merged

Gracefully exit with code 1 when build failed #160

merged 1 commit into from
Jul 5, 2019
+45 −0

Conversation

jooohn
Copy link
Contributor
@jooohn jooohn commented Jun 25, 2019 •
edited
Loading

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
Contributor Author
jooohn commented Jun 25, 2019 •
edited
Loading

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
Pruxis commented Jun 28, 2019

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

@jaredpalmer
Copy link
Owner

Can we make the tests pass?

@jooohn
Copy link
Contributor Author
jooohn commented Jul 1, 2019

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

@Pruxis
Copy link
Pruxis commented Jul 1, 2019

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

@swyxio
Copy link
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
Contributor Author
jooohn commented Jul 1, 2019

basically just retried..

@jaredpalmer jaredpalmer merged commit c850b5c into jaredpalmer:master Jul 5, 2019
@agilgur5 agilgur5 mentioned this pull request Mar 11, 2020
Closed
@agilgur5
Copy link
Collaborator

@allcontributors please add @jooohn for code, tests

@allcontributors allcontributors bot mentioned this pull request Mar 16, 2020
Merged
@allcontributors
Copy link
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
Reviewers

@swyxio swyxio swyxio approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jooohn @Pruxis @jaredpalmer @swyxio @agilgur5