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

Investigate why CI got broken on master but PR passed #9864

Closed
gaearon opened this issue Jun 6, 2017 · 2 comments
Closed

Investigate why CI got broken on master but PR passed #9864

gaearon opened this issue Jun 6, 2017 · 2 comments
Assignees

Comments

@gaearon
Copy link
Collaborator

gaearon commented Jun 6, 2017

See this PR: #9550.

It passed CI on PR, but when merged to master, it had a test failure (which I later fixed in #9854).

I wonder if we're missing some check in PRs.

@trueadm
Copy link
Contributor

trueadm commented Jun 7, 2017

This puzzled me. I didn't understand how the test failed for us now when on the CI run, it ran the test and showed it as passing:

screen shot 2017-06-07 at 11 24 16

Then I remembered that we made a change to make tests properly run on CI a while back. So I think this is related to the CI change we made to the Jest task runner not turning an exit:

https://github.com/facebook/react/blob/7052d7c71071710ad396cf52d3ded9898c3d057d/scripts/tasks/jest.js

@gaearon
Copy link
Collaborator Author

gaearon commented Jun 7, 2017

Thanks for investigating!
To clarify: the issue is that the PR's merge base did not include the fix from #9663.

We might have some more PRs like this, so need to be mindful (and ideally rebase old PRs before merging).

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

No branches or pull requests

2 participants