adjust bld prometheus ext test for concurrent tests, cross namespace … #17635
Conversation
openshift-ci-robot
added
the
size/M
|
/assign @bparees @jim-minter |
| if tc.greaterThan { | ||
| return float64(sample.Value) > tc.value | ||
| if tc.greaterThanEqual { | ||
| return float64(sample.Value) >= tc.value |
There was a problem hiding this comment.
wasn't the issue that we're getting a non-zero value when we expect zero? how does this help that?
There was a problem hiding this comment.
greaterThan was set to false in those cases, so we fell back to the == check below
if you noticed I set greaterThanEqual to true for cancelled / failed
|
lgtm pending passing tests. |
|
tests passed @bparees .. note extended builds was skipped, but since this test is in the prometheus bucket as well, it got picked up by the conformance suite |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bparees, gabemontero The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
openshift-ci-robot
added
the
approved
| } | ||
| return float64(sample.Value) == tc.value | ||
| return float64(sample.Value) < tc.value |
There was a problem hiding this comment.
seems slightly odd that you're changing == to < here, not least given AFAICS there is no metricTest which doesn't have greaterThanEqual set, so this code path isn't actually ever used?
There was a problem hiding this comment.
if you didn't explicitly request greaterThanEqual, then you're implicitly requesting less than... (at least under the new semantics introduced by this PR).
There was a problem hiding this comment.
correct that the code path is not used currently; I wanted the code in place to facilitate usage of greaterThanEqual not being set in the future
and yes, given the precise values are not guarantee-able when this test is run in parallel / in conjunction with other tests, I wanted to have either "greater than or equal" or "less than or equal".
|
/retest |
1 similar comment
|
/retest |
|
/test extended_conformance_install |
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
@gabemontero: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
Automatic merge from submit-queue (batch tested with PRs 17587, 17635). |
…builds
Fixes #17629
@openshift/sig-developer-experience fyi / ptal