logictest: triage remaining tests that block 3node-tenant

[asubiotto]

This PR adds a way to specify an associated issue number when adding a blocklist directive and uses that to associate issues with tests that block 3node-tenant.

A small downside is that all configs from all files are parsed up front, which means that all issues are printed out upfront in the parsing stage. I'm too lazy to plumb this information down to execution time since maybe the correct fix is to not parse all file configs up front, but I think that's out of scope for this PR, so let's either decide to keep the behavior to print issue URLs or remove it if it's annoying.

Release note: None (testing change)

Closes #48798

[cockroach-teamcity]

This change is 

[tbg]

but I'm the wrong person to sign off on logictest UX considerations (the upfront Logfs). Ask a fellow team member please.

Reviewed 1 of 1 files at r1, 1 of 1 files at r2, 20 of 20 files at r3.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @nvanbenschoten)

[asubiotto]

cc @jordanlewis @yuzefovich

[asubiotto]

For reference, this is the example output:

Running make with -j16
GOPATH set to /Users/asubiotto/go
mkdir -p lib
ln -sf /Users/asubiotto/go/native/x86_64-apple-darwin19.4.0/geos/lib/lib{geos,geos_c}.dylib lib
cd ./pkg/sql/logictest && logictest -test.run "TestLogic///" -test.timeout 30m -test.v
I200611 12:56:14.987936 1 rand.go:85  Random seed: -5673526200880651827
=== RUN   TestLogic
    TestLogic: logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/alter_primary_key due to issue: https://go.crdb.dev/issue/50048
    TestLogic: logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/builtin_function due to issue: https://go.crdb.dev/issue/49854
    TestLogic: logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/cascade due to issue: https://go.crdb.dev/issue/50048
    TestLogic: logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/cascade_opt due to issue: https://go.crdb.dev/issue/50048
    TestLogic: logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/crdb_internal due to issue: https://go.crdb.dev/issue/47895
    TestLogic: logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/delete due to issue: https://go.crdb.dev/issue/50048
    TestLogic: logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/enums due to issue: https://go.crdb.dev/issue/50049
    TestLogic: logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/event_log due to issue: https://go.crdb.dev/issue/50047
    TestLogic: logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/family due to issue: https://go.crdb.dev/issue/50048
    TestLogic: logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/fk due to issue: https://go.crdb.dev/issue/49582
    TestLogic: logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/interleaved due to issue: https://go.crdb.dev/issue/49854
    TestLogic: logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/interleaved_join due to issue: https://go.crdb.dev/issue/50050
    TestLogic: logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/lookup_join due to issue: https://go.crdb.dev/issue/49582
    TestLogic: logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/run_control due to issue: https://go.crdb.dev/issue/47895
    TestLogic: logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/schema_change_in_txn due to issue: https://go.crdb.dev/issue/49854
    TestLogic: logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/select_index due to issue: https://go.crdb.dev/issue/49582
    TestLogic: logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/sqlsmith due to issue: https://go.crdb.dev/issue/47899
    TestLogic: logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/system due to issue: https://go.crdb.dev/issue/49854
    TestLogic: logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/system_namespace due to issue: https://go.crdb.dev/issue/48778
    TestLogic: logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/zone_config due to issue: https://go.crdb.dev/issue/49854
    TestLogic: test_log_scope.go:77: test logs captured to: /var/folders/73/qprc_ztn73qfzkyby_nfk1p40000gp/T/logTestLogic964739919
    TestLogic: test_log_scope.go:58: use -show-logs to present logs inline
=== RUN   TestLogic/local
=== RUN   TestLogic/local/aggregate
=== PAUSE TestLogic/local/aggregate
=== RUN   TestLogic/local/alias_types
=== PAUSE TestLogic/local/alias_types
=== RUN   TestLogic/local/alter_column_type
=== PAUSE TestLogic/local/alter_column_type
=== RUN   TestLogic/local/alter_primary_key
=== PAUSE TestLogic/local/alter_primary_key
=== RUN   TestLogic/local/alter_sequence
=== PAUSE TestLogic/local/alter_sequence
=== RUN   TestLogic/local/alter_table
=== PAUSE TestLogic/local/alter_table
=== RUN   TestLogic/local/and_or
=== PAUSE TestLogic/local/and_or
=== RUN   TestLogic/local/apply_join
=== PAUSE TestLogic/local/apply_join
=== RUN   TestLogic/local/array
=== PAUSE TestLogic/local/array
=== RUN   TestLogic/local/as_of
=== PAUSE TestLogic/local/as_of
=== RUN   TestLogic/local/bit
=== PAUSE TestLogic/local/bit
<...>

The will skip lines will be currently printed out even when running a single file make testlogic FILES=<...>

[jordanlewis]

@rafiss are the links emitted here the same as our telemetry links? Is that a problem if our internal devs will also click them, or is it already pretty noisy data so we don't care?

[jordanlewis]

This up front logging is fine by me though, LGTM

[yuzefovich]

The will skip lines will be currently printed out even when running a single file make testlogic FILES=<...>

Hm, so the whole "skip list" is printed out even when running a single logic test file that doesn't skip 3node-tenant config? I think it would be mildly annoying to see it. I find myself running a sile file quite often, so if there is an easy way to reduce unrelated noise, that would be very much appreciated. However, if it's not that trivial or if we think that the skipped logic tests will be unskipped soon, I don't want to block this PR either.

[asubiotto]

Hm, so the whole "skip list" is printed out even when running a single logic test file that doesn't skip 3node-tenant config?

That's correct. I think it might be slightly involved to improve this beyond choosing whether or not we print it.

TFTR. I'm going to merge this since there doesn't seem to be any strong opposition. Happy to change it if people start complaining.

bors r=tbg,jordanlewis

[craig]

Build succeeded

• GitHub CI (Cockroach)

[yuzefovich]

I find this "skipping" output quite annoying, for example:

...
--- total: 29 tests, 0 failures
--- PASS: TestLogic (0.38s)
    logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/alter_primary_key due to issue: https://go.crdb.dev/issue/50048
    logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/builtin_function due to issue: https://go.crdb.dev/issue/49854
    logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/cascade due to issue: https://go.crdb.dev/issue/50048
    logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/cascade_opt due to issue: https://go.crdb.dev/issue/50048
    logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/crdb_internal due to issue: https://go.crdb.dev/issue/47895
    logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/delete due to issue: https://go.crdb.dev/issue/50048
    logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/enums due to issue: https://go.crdb.dev/issue/50049
    logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/event_log due to issue: https://go.crdb.dev/issue/50047
    logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/family due to issue: https://go.crdb.dev/issue/50048
    logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/fk due to issue: https://go.crdb.dev/issue/49582
    logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/interleaved due to issue: https://go.crdb.dev/issue/49854
    logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/interleaved_join due to issue: https://go.crdb.dev/issue/50050
    logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/lookup_join due to issue: https://go.crdb.dev/issue/49582
    logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/run_control due to issue: https://go.crdb.dev/issue/47895
    logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/schema_change_in_txn due to issue: https://go.crdb.dev/issue/49854
    logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/select_index due to issue: https://go.crdb.dev/issue/49582
    logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/sqlsmith due to issue: https://go.crdb.dev/issue/47899
    logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/system due to issue: https://go.crdb.dev/issue/49854
    logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/system_namespace due to issue: https://go.crdb.dev/issue/48778
    logic.go:1452: will skip 3node-tenant config in test testdata/logic_test/zone_config due to issue: https://go.crdb.dev/issue/49854
    test_log_scope.go:77: test logs captured to: /var/folders/9s/y2m91fyd5vj09d19mf4g8p6w0000gp/T/logTestLogic970011979
    test_log_scope.go:58: use -show-logs to present logs inline
    --- PASS: TestLogic/local (0.29s)
        --- PASS: TestLogic/local/vectorize_local (0.28s)
    --- PASS: TestLogic/local-vec-off (0.00s)
    --- PASS: TestLogic/[email protected] (0.00s)
    --- PASS: TestLogic/local-vec-auto (0.00s)
    --- PASS: TestLogic/fakedist (0.00s)
    --- PASS: TestLogic/local-mixed-19.2-20.1 (0.00s)
    --- PASS: TestLogic/fakedist-vec-off (0.00s)
    --- PASS: TestLogic/fakedist-vec-auto (0.00s)
    --- PASS: TestLogic/fakedist-vec-auto-disk (0.00s)
    --- PASS: TestLogic/fakedist-metadata (0.00s)
    --- PASS: TestLogic/fakedist-disk (0.00s)
    --- PASS: TestLogic/5node (0.00s)
    --- PASS: TestLogic/5node-vec-auto (0.00s)
    --- PASS: TestLogic/5node-vec-disk-auto (0.00s)
    --- PASS: TestLogic/5node-metadata (0.00s)
    --- PASS: TestLogic/5node-disk (0.00s)
    --- PASS: TestLogic/3node-tenant (0.00s)
PASS

I believe this logging should be off by default because most engineers don't care about the skipping, but this verbose output adds additional burden on reading the test output. I'm in favor of changing the default behavior to not print this skipping info.

[rafiss]

I don't think using the shortlinks here will create too much extra noise.

[asubiotto]

Moved this logging behind a flag: #50267