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

[FR]: Support timeout option #95

Closed
1 task
Aghassi opened this issue Jan 30, 2023 · 2 comments · Fixed by #110
Closed
1 task

[FR]: Support timeout option #95

Aghassi opened this issue Jan 30, 2023 · 2 comments · Fixed by #110
Labels
enhancement New feature or request

Comments

@Aghassi
Copy link

Aghassi commented Jan 30, 2023

What is the current behavior?

Currently there is no timeout attribute on this rule. Internally, we have some references to rules_nodejs jest_test we wrote that supports passing the timeout attribute to the rule https://bazel.build/reference/be/common-definitions#test.timeout. This is particularly useful for longer running targets until we migrate them to be smaller. Having this would ease our migration path

Describe the feature

Given a jest_test rule, take an attribute called timeout that passes along the values described here https://bazel.build/reference/be/common-definitions#test.timeout

Fund our work

@Aghassi Aghassi added the enhancement New feature or request label Jan 30, 2023
@alexeagle
Copy link
Member

This seems to be working as intended for me. jest_test passes **kwargs to the underlying js_test and timeout gets included that way.

At HEAD (e6e9cfd) I made one test take over a minute. With timeout=short Bazel reports a timeout, and with timeout=moderate it succeeds.

Here's my repro:
https://github.com/aspect-build/rules_jest/compare/main..i95

@alexeagle
Copy link
Member

okay I pushed another commit to that branch, it's only reproducible with snapshots enabled.

alexeagle added a commit that referenced this issue Mar 21, 2023
Fixes #95

BREAKING CHANGE:
The default timeout for jest_test is now short (1m by default) rather than moderate (5m by default).
Tests that take more than 1m should have a size or timeout attribute added.
alexeagle added a commit that referenced this issue Mar 21, 2023
Fixes #95

BREAKING CHANGE:
The default timeout for jest_test is now short (1m by default) rather than moderate (5m by default).
Tests that take more than 1m should have a size or timeout attribute added.
alexeagle added a commit that referenced this issue Mar 21, 2023
Fixes #95

BREAKING CHANGE:
The default timeout for jest_test is now short (1m by default) rather than moderate (5m by default).
Tests that take more than 1m should have a size or timeout attribute added.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants