bazel icon indicating copy to clipboard operation
bazel copied to clipboard

Test timeout warnings are too strict

Open daveah opened this issue 7 years ago • 10 comments

Description of the problem / feature request:

When setting the test timeout there is a strict upper-bound and a weakly enforced lower bound. If the test runs below the lower bound then a warning is issued. There should be a way to better configure this range such that warnings are never issued.

Feature requests: what underlying problem are you trying to solve with this feature?

I have a test which reads a lot of data from disk. If that data happens to be in cache, the test runs relatively quickly (for example 100-150s), but if the data isn't in cache the test can run much slower (for example 400-500s). Due to the latter case, I have to set timeout="long". But when it runs quickly I get a warning that is confusing to any other users. There should be a way to set the lower bound such that warnings are squelched over a wider range.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Run a simple test with randomly selected sleep 100s or sleep 400s.

What operating system are you running Bazel on?

Red Hat EL6

What's the output of bazel info release?

development version

If bazel info release returns "development version" or "(@non-git)", tell us how you built Bazel.

What's the output of git remote get-url origin ; git rev-parse master ; git rev-parse HEAD ?

Have you found anything relevant by searching the web?

Unable to find anything on this particular issue

Any other information, logs, or outputs that you want to share?

.

daveah avatar Apr 13 '18 14:04 daveah

Should there be a warning at all? Does the timeout affect the execution in any way? If not, why warn about the wrong expectation?

My usecase is local execution during development vs execution on the ci server which is slower. Some small tests therefore run into the timeout on the ci server. Increasing the timeouts for just these tests to make it work on the server still yields warnings when running locally which is annoying.

The only way to get rid of the warnings seems to be to increase the timeout for all (small) tests which seems questionable.

marcohu avatar Apr 20 '18 09:04 marcohu

I guess extending --test_timeout to support specifying the lower bound makes sense... but I haven't been able to get the warning you are talking about by running a test with a short sleep in it and setting timeout=long. How do you get the warning? And what does the warning exactly say?

jmmv avatar Sep 11 '19 17:09 jmmv

I'm not sure whether this still happens. I stopped using Bazel and switched to cmake sometime around 12 months ago due to unrelated issues I was having the Bazel's lack of decent support for building and testing Python C++ bindings. As such, I can't say whether this issue still occurs or not. Not sure whether marcohu who commented on this last April shortly after I raised the issue still sees it or not.

daveah avatar Sep 11 '19 20:09 daveah

I have a similar problem where the test times differ a lot between configurations (release/debug/sanitize) and platform (Linux/Windows). In my case, allowing select() to set the timeout would be enough.

The general case described in this issue is also interesting, where some targets needs to be treated in a special way. Possible solutions I can come up with are tagging the test for not comparing lower bound at all or to manually set a lower bound. Both variants would also benefit if select() would be allowed.

moroten avatar Oct 09 '19 12:10 moroten

@moroten Can you provide the exact warning you are seeing? As I mentioned earlier, I could not reproduce this...

jmmv avatar Oct 14 '19 16:10 jmmv

What I do is

touch test.sh
chmod +x test.sh
echo 'sh_test(
    name = "test",
    size = "small",
    srcs = ["test.sh"],
    timeout = "moderate",  # The test runs quickly on Linux, but needs "moderate" on Windows
    visibility = ["//visibility:public"],
)' > BUILD.bazel
bazel test --test_verbose_timeout_warnings test

Which results in

//:test                                                         PASSED in 0.1s
  WARNING: //:test: Test execution time (0.1s excluding execution overhead) outside of range for MODERATE tests. Consider setting timeout="short" or size="small".

Changing the timeout field to timeout = select({"@platforms//os:windows": "moderate", "//conditions:default": "short"}) results in

ERROR: BUILD.bazel:1:1: //:test: attribute "timeout" is not configurable

moroten avatar Oct 15 '19 11:10 moroten

Ah, found it. This comes from TestResultAggregator.java.

jmmv avatar Oct 17 '19 15:10 jmmv

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 3 years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

github-actions[bot] avatar Feb 28 '23 01:02 github-actions[bot]

@bazelbuild/triage not stale

brentleyjones avatar Feb 28 '23 14:02 brentleyjones

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 90 days unless any other activity occurs. If you think this issue is still relevant and should stay open, please post any comment here and the issue will no longer be marked as stale.

github-actions[bot] avatar May 04 '24 01:05 github-actions[bot]