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 flaky @babel/cli test #14385

Merged
merged 8 commits into from Mar 27, 2022
Merged

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Mar 23, 2022

Q                       A
Fixed Issues? The @babel/cli watch test is flaky because lack of inter-process communication
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

To re-run the @babel/cli test again and again, I have pushed some CI maintenance commits.

@babel-bot
Copy link
Collaborator

babel-bot commented Mar 23, 2022

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/51553/

@JLHwung JLHwung marked this pull request as ready for review March 23, 2022 20:11
@nicolo-ribaudo
Copy link
Member

I love this PR

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

This might still not solve it because it just "waits more", but 2s should be more then enough 👍

- name: 'Check for unmet constraints (fix w/ "yarn constraints --fix")'
run: |
yarn constraints
- name: 'Check for duplicate dependencies (fix w/ "yarn dedupe")'
if: steps.yarn-cache.outputs.cache-hit != 'true'
Copy link
Member

Choose a reason for hiding this comment

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

We do we always run it now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. I should have left a comment here. This step was previously in the prepare-yarn-cache job including the step yarn-cache. Not long ago it was extracted to a separate job yarn-validate which can't access the yarn-cache output anymore, so the condition here is always true now. The dedupe check is around 2 seconds so we can just leave it here.

@nicolo-ribaudo nicolo-ribaudo added the PR: Internal 🏠 A type of pull request used for our changelog categories label Mar 23, 2022
@nicolo-ribaudo nicolo-ribaudo merged commit fe00618 into babel:main Mar 27, 2022
@nicolo-ribaudo nicolo-ribaudo deleted the fix-flaky-babel-cli-test branch March 27, 2022 10:46
@nicolo-ribaudo
Copy link
Member

I'm merging this with a single review because it doesn't affect any source code, and it fixes a super annoying problem that makes ~30% of CI runs fail.

@liuxingbaoyu
Copy link
Member

There seems to be a failure for the same reason.

We can consider adding a watch ready output.

Or consider the following changes:

  await new Promise(resolve => setTimeout(resolve, 2000));
  fs.writeFileSync("./file.txt", "Updated!");
 let i = 0;
  setInterval(() => {
    fs.writeFileSync("./file.txt", `Updated${i++}!`);
  }, 300);

@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jul 15, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Internal 🏠 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants