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

fix: correct check for compatible `find` binary #10346

Merged
merged 4 commits into from Jul 31, 2020
Merged

Conversation

@SimenB
Copy link
Collaborator

SimenB commented Jul 31, 2020

Summary

Fixes #10339

Test plan

Verified on Windows VM. Not sure how to write a test for this… I'm quite confused by why our windows tests pass without this - I doubt watchman is installed. Maybe they have some other find?

@SimenB
Copy link
Collaborator Author

SimenB commented Jul 31, 2020

FYI @richardlau 🙂

@richardlau
Copy link
Contributor

richardlau commented Jul 31, 2020

Verified on Windows VM. Not sure how to write a test for this… I'm quite confused by why our windows tests pass without this - I doubt watchman is installed. Maybe they have some other find?

The test mocks child_process so doesn't actually spawn.

jest.mock('child_process', () => ({
spawn: jest.fn((cmd, args) => {
let closeCallback;
return {
on: jest.fn().mockImplementation((event, callback) => {
if (event === 'exit') {
callback(mockSpawnExit, null);
}
}),
stdout: {
on: jest.fn().mockImplementation((event, callback) => {
if (event === 'data') {
setTimeout(() => {
callback(mockResponse);
setTimeout(closeCallback, 0);
}, 0);
} else if (event === 'close') {
closeCallback = callback;
}
}),
setEncoding: jest.fn(),
},
};
}),
}));

@SimenB
Copy link
Collaborator Author

SimenB commented Jul 31, 2020

Verified on Windows VM. Not sure how to write a test for this… I'm quite confused by why our windows tests pass without this - I doubt watchman is installed. Maybe they have some other find?

The test mocks child_process so doesn't actually spawn.

Right, but we test Jest using Jest itself, and spawn an instance of Jest in every e2e test. So I'd have thought CI would fail when trying to run any tests, not these specific unit tests.

@SimenB SimenB merged commit 2ab3fb2 into facebook:master Jul 31, 2020
20 of 22 checks passed
20 of 22 checks passed
cleanup-runs
Details
Running TypeScript compiler & ESLint
Details
Node v10.x on ubuntu-latest
Details
Node v10.x on macOS-latest
Details
Node v10.x on windows-latest
Details
Node v12.x on ubuntu-latest
Details
Node v12.x on macOS-latest
Details
Node v12.x on windows-latest
Details
Node v13.x on ubuntu-latest
Details
Node v13.x on macOS-latest
Details
Node v13.x on windows-latest
Details
Node v14.x on ubuntu-latest
Details
Node v14.x on macOS-latest
Details
Node v14.x on windows-latest
Details
ci/circleci: test-node-14 Your tests failed on CircleCI
Details
ci/circleci: test-node-12 CircleCI is running your tests
Details
Facebook CLA Check Contributor License Agreement is valid!
Details
ci/circleci: test-jest-circus Your tests passed on CircleCI!
Details
ci/circleci: test-node-10 Your tests passed on CircleCI!
Details
ci/circleci: test-node-13 Your tests passed on CircleCI!
Details
ci/circleci: test-or-deploy-website Your tests passed on CircleCI!
Details
facebook.jest #20200731.8 succeeded
Details
@SimenB SimenB deleted the SimenB:find-windows branch Jul 31, 2020
@MikeActually MikeActually mentioned this pull request Aug 4, 2020
5 of 9 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.