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: Set size to a default value as well as timeout. #839

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

Conversation

matts1
Copy link

@matts1 matts1 commented May 13, 2024

Currently, we are unable to run our write_source_files tests in our pre-upload checks, because we have --test_size_filter=small, and setting size will attempt to set it on both the run rule and the test rule, the former being invalid.

See here for more details.

Changes are visible to end-users: no

Test plan

  • Manual testing; please provide instructions so we can reproduce:
    bazel test //... --test_size_filters=small

Before: 5 tests run
After: 152 tests run

@CLAassistant
Copy link

CLAassistant commented May 13, 2024

CLA assistant check
All committers have signed the CLA.

@matts1 matts1 changed the title Set size to a default value as well as timeout. fix: Set size to a default value as well as timeout. May 13, 2024
lib/private/utils.bzl Outdated Show resolved Hide resolved
Currently, we are unable to run our `write_source_files` tests in our pre-upload checks, because we have `--test_size_filter=small`, and setting `size` will attempt to set it on both the run rule and the test rule, the former being invalid.
@matts1
Copy link
Author

matts1 commented May 13, 2024

I just realized a better way to do it.

Previously:

  • Neither specified -> size = "medium", timeout = "short"
  • Timeout only -> size = "medium", timeout = timeout
  • size only -> size = size, timeout = inferred by bazel based on size
  • both -> size = size, timeout = timeout

Now:

  • (changed) Neither specified -> size = "small", timeout = inferred by bazel based on size (so always short)
  • (changed) Timeout only -> size = "small", timeout = timeout
  • size only -> size = "size", timeout = inferred by bazel based on size
  • both -> size = size, timeout = timeout

@matts1 matts1 requested a review from rrbutani May 19, 2024 23:19
@matts1
Copy link
Author

matts1 commented May 21, 2024

Any chance we can get this merged?

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.

None yet

3 participants