Skip to content

Comments

ci: Add --test_verbose_timeout_warnings#27150

Merged
phlax merged 1 commit intoenvoyproxy:mainfrom
phlax:ci-verbose-timeout-warnings
May 3, 2023
Merged

ci: Add --test_verbose_timeout_warnings#27150
phlax merged 1 commit intoenvoyproxy:mainfrom
phlax:ci-verbose-timeout-warnings

Conversation

@phlax
Copy link
Member

@phlax phlax commented May 3, 2023

Its not clear what this flag does - the docs say it outputs warnings about "tests that are too big"

currently we get such warnings without any useful info, just advice to add this flag

in my testing adding this flag does nothing other than make the warnings go away - seems worth adding

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

@phlax phlax force-pushed the ci-verbose-timeout-warnings branch from 65db630 to 15b31e2 Compare May 3, 2023 11:58
@phlax
Copy link
Member Author

phlax commented May 3, 2023

reading here https://docs.aspect.build/tutorial-paywall/testing/#the-encyclopedia and iiuc the default test size is "moderate" and this should tell you that tests that can run quickly dont need that

@phlax phlax force-pushed the ci-verbose-timeout-warnings branch 2 times, most recently from 443a7f0 to a04bd1f Compare May 3, 2023 12:16
@phlax
Copy link
Member Author

phlax commented May 3, 2023

my reading of bazelbuild/bazel#13103 is that this flag may not work correctly

its kinda nice not getting the useless warnings about the useless warning flag, but it would be good to get a read on which tests are "too big"

Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax phlax force-pushed the ci-verbose-timeout-warnings branch from f3c61b0 to 2e105cb Compare May 3, 2023 13:35
@phlax
Copy link
Member Author

phlax commented May 3, 2023

i tried testing this with various --test_output settings which made no difference, afaict this flag does the ~opposite of what it says, but maybe im missing something

@keith
Copy link
Member

keith commented May 3, 2023

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

here's an example of this warning. in this case i made foo large

@phlax
Copy link
Member Author

phlax commented May 3, 2023

i see - i saw similar in posted logs, but didnt seem to get this in any of the tests (which otherwise emit the warning about the warning flag)

either way, i reckon this change should be good

@phlax phlax enabled auto-merge (squash) May 3, 2023 16:07
@phlax phlax merged commit 3fe4b8d into envoyproxy:main May 3, 2023
phlax added a commit to phlax/envoy that referenced this pull request May 21, 2023
Signed-off-by: Ryan Northey <ryan@synca.io>
phlax added a commit that referenced this pull request May 22, 2023
Signed-off-by: Ryan Northey <ryan@synca.io>
reskin89 pushed a commit to reskin89/envoy that referenced this pull request Jul 11, 2023
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Eskin <ryan.eskin89@protonmail.com>
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.

2 participants