Skip to content

feat: Add require-to-pass-timeout rule (#345) #346

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gustavo-meilus
Copy link

This PR introduces a rule that enforces the requirement of specifying a timeout for the toPass() function. The toPass() function is used to retry blocks of code until they pass successfully. However, if no timeout is defined, the test may run indefinitely until the overall test timeout is reached.
By requiring a timeout, this rule ensures that tests will require a timeout on toPass() to be able to fail early.

@gustavo-meilus gustavo-meilus force-pushed the rule/require-to-pass-timeout branch 2 times, most recently from 41eeb60 to f69316a Compare May 6, 2025 23:57
@mskelton
Copy link
Member

Hey @gustavo-meilus, so sorry for the delay on this. I'm not really sure how I feel about this rule. It's a very specific opinion as far as requiring this rule. I'm not inclined to add a lot more to the plugin than we need, and given there have been no upvotes or comments on the original issue, I'm inclined to close this.

@MSroczynski3
Copy link
Contributor

MSroczynski3 commented Jul 31, 2025

I can understand both points of view. I am more surprised that this method has no default timeout, and it is even clearly stated in the documentation:

Note that by default toPass has timeout 0 and does not respect custom [expect timeout].
(https://playwright.dev/docs/next/test-timeouts#expect-timeout).

I would suggest opening an issue on the Playwright repository asking about this.

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