logictest: skip distsql_tenant_locality under duress
#118769
Conversation
|
This change is |
rickystewart
force-pushed
the
skip-distsql-tenant-locality-test
branch
from
81152b7
to
3b5f45f
Compare
There was a problem hiding this comment.
You beat me to it! Thanks, sorry I didn't do this earlier.
Reviewed 1 of 12 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @rickystewart)
pkg/cmd/generate-logictest/templates.go line 76 at r1 (raw file):
defer sql.TestingOverrideExplainEnvVersion("CockroachDB execbuilder test version")() if file == "distsql_tenant_locality" { skip.WithIssue(t, 118627)
We've only seen the failures under stress. Would skip.UnderStress be more targeted?
There was a problem hiding this comment.
Reviewable status:
pkg/cmd/generate-logictest/templates.go line 76 at r1 (raw file):
Previously, michae2 (Michael Erickson) wrote…
We've only seen the failures under stress. Would
skip.UnderStressbe more targeted?
I second this - I think skipping under stress is the right approach (some context is in #118559 (comment)).
|
We should also revert 051191c as part of the skip. |
|
How about skipping under duress? That would include stress as well as |
|
Sounds good to me. |
rickystewart
force-pushed
the
skip-distsql-tenant-locality-test
branch
from
3b5f45f
to
0b63d1a
Compare
rickystewart
changed the title
distsql_tenant_localitydistsql_tenant_locality under duress
|
Is there a way we could put the skip information into the |
AFAIK logictests have no "skip under X condition" directives or anything like that with functionality corresponding to the |
There was a problem hiding this comment.
Reviewed 13 of 13 files at r2, all commit messages.
Reviewable status:
pkg/sql/opt/exec/execbuilder/testdata/distsql_tenant_locality line 55 at r2 (raw file):
# - TableReader on SQL Instance 3 to scan Span /106/1/3/0 # - TableReader on SQL Instance 1 to scan Span /106/1/5/0. query T retry
nit: we should remove retry here too (it was added in the previous attempt at fixing the test 85439c3).
Flaky test: see cockroachdb#118627 Epic: None Release note: None
rickystewart
force-pushed
the
skip-distsql-tenant-locality-test
branch
from
0b63d1a
to
f90b91b
Compare
|
Should be good now. |
|
TFTRs! bors r=yuzefovich,michae2 |
|
Build failed (retrying...): |
|
Build succeeded: |
In cockroachdb#118769 we added a skip under duress for the distsql_tenant_locality logic test due to cockroachdb#118627. Now that the `skip` command supports calling `skip.UnderDuress()`, move the skip for distsql_tenant_locality from the logictest template to the test script itself. Informs: cockroachdb#118627 Epic: None Release note: None
118820: logictest: add ability to call skip.UnderStress from Test-script r=rafiss,rickystewart a=michae2 **logictest: add ability to call skip.UnderStress from Test-script** We already had a `skip` command in our fork of Test-script. Flesh it out a bit with options for `skip.UnderStress` and family. The new syntax comes in three forms: ``` skip <ISSUE> [args...] skip ignorelint [args...] skip under <deadlock/race/stress/stressrace/metamorphic/duress> [ISSUE] [args...] ``` The first form calls `skip.WithIssue`, the second form calls `skip.IgnoreLint`, and the third form calls `skip.UnderStress` and family. Before this change, the only supported form was `skip [ISSUE]` which called `skip.IgnoreLint`. With this cange, commands in this original form will instead call `skip.WithIssue`. AFAIK there are no existing skip commands currently, so this should be a moot point. Also fix one mistaken use of `skip` which should have been `skipif`. Informs: #118627 Epic: None Release note: None --- **exec: skip distsql_tenant_locality under duress** In #118769 we added a skip under duress for the distsql_tenant_locality logic test due to #118627. Now that the `skip` command supports calling `skip.UnderDuress()`, move the skip for distsql_tenant_locality from the logictest template to the test script itself. Informs: #118627 Epic: None Release note: None Co-authored-by: Michael Erickson <michae2@cockroachlabs.com>
In cockroachdb#118769 we added a skip under duress for the distsql_tenant_locality logic test due to cockroachdb#118627. Now that the `skip` command supports calling `skip.UnderDuress()`, move the skip for distsql_tenant_locality from the logictest template to the test script itself. Informs: cockroachdb#118627 Epic: None Release note: None
Flaky test: see #118627
Epic: None
Release note: None