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

Merged
merged 3 commits into from
Jul 19, 2024

Conversation

matts1
Copy link
Contributor

@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
Contributor 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

nya3jp pushed a commit to nya3jp/cros-bazel that referenced this pull request May 14, 2024
Unfortunately, //bazel:generate_cargo_files_test does not run because it's not marked as small, and there's no way to mark it as small.
This should be fixed by bazel-contrib/bazel-lib#839

BUG=None
TEST=portage/tools/run_tests.sh

Change-Id: Iea7b85df7d59b6f0bfaa6973a4e41119d9d6c102
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/bazel/+/5531394
Auto-Submit: Matt Stark <msta@google.com>
Reviewed-by: Shuhei Takahashi <nya@chromium.org>
Commit-Queue: Shuhei Takahashi <nya@chromium.org>
Tested-by: Matt Stark <msta@google.com>
@matts1 matts1 requested a review from rrbutani May 19, 2024 23:19
@matts1
Copy link
Contributor Author

matts1 commented May 21, 2024

Any chance we can get this merged?

@alexeagle alexeagle enabled auto-merge (squash) July 19, 2024 19:50
@alexeagle alexeagle merged commit 59453e5 into bazel-contrib:main Jul 19, 2024
24 checks passed
@matts1 matts1 deleted the size branch July 23, 2024 00:34
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.

4 participants