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

Exit yarn bootstrap with nonzero code if failed #19089

Merged
merged 1 commit into from
Sep 2, 2022
Merged

Conversation

IanVS
Copy link
Member

@IanVS IanVS commented Sep 2, 2022

Issue: CI fails in a non-obvious way if yarn bootstrap --build fails

What I did

When one or more of the steps yarn bootstrap --build fails, the exit code was 0, so CI doesn't know anything went wrong, and then random-seeming steps further on in the process fail. For an example of this, see https://app.circleci.com/pipelines/github/storybookjs/storybook/28842/workflows/d3da014b-c899-4113-890d-80141f32fc11. You'll note that the build step is green, but should have failed.

So, this change returns the spawnSync result back to the spot where it's being called, then we check for a non-zero status code and process.exit() with that code if necessary.

How to test

Make a change that causes yarn bootstrap --build to fail, then echo $?. You should see a non-zero number after this PR, whereas before it was 0.

@IanVS IanVS added the maintenance User-facing maintenance tasks label Sep 2, 2022
@IanVS IanVS requested a review from shilman September 2, 2022 03:28
# Conflicts:
#	scripts/bootstrap.js
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Good change 💯

@shilman shilman merged commit a386695 into next Sep 2, 2022
@shilman shilman deleted the bootstrap-exit-code branch September 2, 2022 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants