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

test: add env variable TEST_IGNORE_TIMEOUT_FAILURES to suppress timeo… #4543

Merged
merged 1 commit into from
Jun 9, 2022

Conversation

edvardchen
Copy link
Contributor

@edvardchen edvardchen commented Jun 9, 2022

…ut failures

What's the problem this PR addresses?

In order to pass citgm, we need to suppress timeout failures. See the discussion thread nodejs/citgm#905 (comment)

How did you fix it?

To add env variable TEST_IGNORE_TIMEOUT_FAILURES to turn on this kind of suppression

Checklist

  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

Copy link
Member

@merceyz merceyz left a comment

Choose a reason for hiding this comment

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

I would rather we try to figure out why the timeouts happen, if Node.js introduces changes that causes timeouts they wont be caught with this.

@arcanis
Copy link
Member

arcanis commented Jun 9, 2022

@merceyz I think the priority is to get CITGM to work with Yarn in its current state, as most of the problems will be caught even without catching timeouts (regressions, empty event loops, ... the process hanging for real is possible, but only one very unlikely symptoms amongst many).

Additionally, debugging it on their infra seems a real pain as tests have to be run manually by someone else, so it's hard to estimate how long it'd take to find the problem, and is probably out-of-scope for a first-time contribution.

@merceyz
Copy link
Member

merceyz commented Jun 9, 2022

@arcanis That's fair, I retract my objection.

@merceyz merceyz merged commit 617aa2e into yarnpkg:master Jun 9, 2022
@merceyz
Copy link
Member

merceyz commented Jun 9, 2022

I've triggered a release for this, should be ready soon.
https://github.com/yarnpkg/berry/actions/runs/2468984118

merceyz pushed a commit that referenced this pull request Oct 22, 2023
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

Successfully merging this pull request may close these issues.

3 participants