Flip --incompatible_check_sharding_support

[fmeum]

RELNOTES[INC]: --incompatible_check_sharding_support is enabled by default. Sharded tests with test runners that do not properly advertise support for test sharding will fail. Refer to #18339 for migration advice.

[meteorcloudy]

The current downstream pipeline is not in a great state, I'll approve this as soon as I think it's ready.

[meteorcloudy]

@fmeum Can you rebase this PR to Bazel HEAD and I'll start a downstream testing for this change?

[fmeum]

@meteorcloudy Updated

[meteorcloudy]

Thanks, here we go: https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/3028

[meteorcloudy]

More tests in Bazel are broken: https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/3028#018823f5-fcb7-42d0-8ef8-7d5087a6a6e7 😅

[meteorcloudy]

But the strings_test, sharding did work:

pcloudy@pcloudy-macbookpro3:~/workspace/my_tests/bazel (master)
$ cat bazel-testlogs/src/test/cpp/util/strings_test/shard_1_of_2/test.log
exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //src/test/cpp/util:strings_test
-----------------------------------------------------------------------------
Running main() from gmock_main.cc
Note: This is test shard 1 of 2.
[==========] Running 4 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 4 tests from BlazeUtil
[ RUN      ] BlazeUtil.JoinStrings
[       OK ] BlazeUtil.JoinStrings (0 ms)
[ RUN      ] BlazeUtil.Replace
[       OK ] BlazeUtil.Replace (0 ms)
[ RUN      ] BlazeUtil.Tokenize
[       OK ] BlazeUtil.Tokenize (0 ms)
[ RUN      ] BlazeUtil.StringPrintf
[       OK ] BlazeUtil.StringPrintf (0 ms)
[----------] 4 tests from BlazeUtil (0 ms total)

[----------] Global test environment tear-down
[==========] 4 tests from 1 test suite ran. (0 ms total)
[  PASSED  ] 4 tests.
pcloudy@pcloudy-macbookpro3:~/workspace/my_tests/bazel (master)
$ cat bazel-testlogs/src/test/cpp/util/strings_test/shard_2_of_2/test.log
exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //src/test/cpp/util:strings_test
-----------------------------------------------------------------------------
Running main() from gmock_main.cc
Note: This is test shard 2 of 2.
[==========] Running 4 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 4 tests from BlazeUtil
[ RUN      ] BlazeUtil.Split
[       OK ] BlazeUtil.Split (0 ms)
[ RUN      ] BlazeUtil.StripWhitespace
[       OK ] BlazeUtil.StripWhitespace (0 ms)
[ RUN      ] BlazeUtil.SplitQuoted
[       OK ] BlazeUtil.SplitQuoted (0 ms)
[ RUN      ] BlazeUtil.EndsWithTest
[       OK ] BlazeUtil.EndsWithTest (0 ms)
[----------] 4 tests from BlazeUtil (0 ms total)

[----------] Global test environment tear-down
[==========] 4 tests from 1 test suite ran. (0 ms total)
[  PASSED  ] 4 tests.

Not sure why the check failed.

[meteorcloudy]

https://github.com/search?q=repo%3Agoogle%2Fgoogletest%20TEST_SHARD_STATUS_FILE&type=code, is it because it's called GTEST_SHARD_STATUS_FILE?

[meteorcloudy]

Oh no, I found the following copybara transformation for googletests:

        "TEST_SHARD_INDEX": "GTEST_SHARD_INDEX",
        "TEST_SHARD_STATUS_FILE": "GTEST_SHARD_STATUS_FILE",
        "TEST_TOTAL_SHARDS": "GTEST_TOTAL_SHARDS",

[fmeum]

I saw that variant while working on this PR and already wondered how it could work - turns out it doesn't?

[meteorcloudy]

turns out it doesn't?

I think so, unless we also pass and check those env vars?

[fmeum]

@meteorcloudy I pushed two commits that should fix this, could you run the downstream test again?

[meteorcloudy]

Here you go: https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/3035

But I guess it's better to submit the fixes in a different PR so that we can also cherry pick them for 6.3.0.

[fmeum]

But I guess it's better to submit the fixes in a different PR so that we can also cherry pick them for 6.3.0.

Will do, but the flip PR is really the only way for me to test whether they are indeed effective.

[meteorcloudy]

Thanks! We can always stack the flag flip on top of the fix to test, just like this one ;)

[meteorcloudy]

Looks like only Kythe was broken due to the flag flip: https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/3035#0188343c-bd18-4174-a702-55c777bce2e3

[fmeum]

rules_go didn't declare support correctly and while the issue is fixed at HEAD, we still need to have a release.

[meteorcloudy]

@fmeum Can you submit the fixes as a separate PR, then we can merge and cherry pick that one, then flip the flag at HEAD.

[fmeum]

@meteorcloudy Submitted #18469

[meteorcloudy]

@fmeum Thanks, #18469 is merged, please rebase and we can also merge this one ;)

[fmeum]

@meteorcloudy Done