Turn on racetest for all presubmits#3780
Conversation
|
@xiaolanz PR needs rebase |
|
looks like there are still failures? |
|
Looks like some tests are being flaking. /retest |
|
circle doesn't understand "/retest" you have to click the button in the UI |
Automatic merge from submit-queue. fix race test for TestWatchChanges address: #3780 go test --v ./mixer/pkg/runtime2/... --race -run TestWatchChanges
Codecov Report
@@ Coverage Diff @@
## master #3780 +/- ##
=======================================
- Coverage 71% 70% -<1%
=======================================
Files 317 317
Lines 28911 29516 +605
=======================================
+ Hits 20247 20581 +334
- Misses 7482 7693 +211
- Partials 1182 1242 +60
Continue to review full report at Codecov.
|
|
@xiaolanz PR needs rebase |
|
PTAL |
|
/retest istio-unit-tests |
| - e2e-simple: | ||
| requires: | ||
| - build | ||
| - racetest: |
There was a problem hiding this comment.
No longer need dependencies - but you may run it after test.
|
/approve |
costinm
left a comment
There was a problem hiding this comment.
Please don't add it to presubmit until it is consistently greed for few days.
It failed 3 hours ago in the flaky suite.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kyessenov, sebastienvas The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
New changes are detected. LGTM label has been removed. |
|
@costinm this turns on for presubmit but doesn't block. So i can find the flakies. |
|
We should NEVER move a whole suite to flaky when only one test is flaky. Instead we should create an ignore file, where you put the flaky test, and file a bug when this happen. Preventing all the tests from running is just an open doors for more coming bugs. |
|
Note that you can skip a test for flaky directly in the test which is even better than the ignore file. |
|
Yes - it's fine to ignore/skip tests from the suite if a bug is filled. But so far this has not been done for Where the test is running should have no impact on fixing the bugs - all tests should upload to the The cirlceCI presubmit is intended to contain a subset of the tests suites, with <15 min run time and very reliable. There are 2 periodic runners and a nightly build, running other suites - and prow. There are cases where the presubmit will not catch bugs that are covered by longer-running suites - passsing |
|
Let's discuss this elsewhere. The 15 mn requirement only makes sense when you know what are testing for. Your presubmit time should be defined by the % of postsubmit failure not just by a random number. 15mn is something we want to reach eventually, but not a hard stop. |
|
back to the topic of the PR, racetest is still failing: for instance https://circleci.com/gh/istio/istio/52665 |
|
This is not enforced so new tests can break racetest. TestKeyCertBundleRotator is a regression. Filed bug #4385 |
|
if it's not enforced it'll get back or stay red forever. though conversely it has to be non flaky green to be enforced. I know you are fighting a tide there joy, but you're almost there... keep pushing! |
|
Yeah, it's a deliberate art to find and exclude all flaky tests while identifying and fixing the real regressions in a timely fashion. |
addresses #4235
We will watch submission queue for a few days before turn on "required"