Adar Dembo has posted comments on this change. ( )

Change subject: build: enable sharding within cmake/ctest

Patch Set 2:


Makes sense, I'm much happier to see the sharding definitions colocated with  
test definitions.
Commit Message:
PS2, Line 26: * Flaky-test tracking is currently still done by the test binary 
            :   and not the specific shard, though we could easily switch that 
            :   the future.
This gets weird though because the number of shards may change over time, which 
would muck up the sharded test's history.
File CMakeLists.txt:
PS2, Line 678: If a test suite is long enough
             : #       to require a bumped timeout, consider enabling sharding 
of the
             : #       test by adding it to the NUM_SHARDS_BY_TEST dictionary 
File build-support/
PS2, Line 56: TEST_COMMAND_RE = re.compile('Test command: (.+)$')
            : TEST_ENV_RE = re.compile('^\d+:  (\S+)=(.+)')
            : LDD_RE = re.compile(r'^\s+.+? => (\S+) \(0x.+\)')
Would be nice to include an example string for each of these.
PS2, Line 322:                       name=execution.test_name,
Note that prior to your change the name of the test also included the total 
number of shards. Now it doesn't anymore. Does that matter, or was it purely 
PS2, Line 442:   execution = dict(argv=(["", options.cmd] + 
options.args), env={})
This isn't right, I think it should be:

  execution = TestExecution([...] + options.args)

Something like that? Actually, this should still call get_test_executions() in 
some capacity, right? Otherwise looping a sharded test will ignore the sharding.

To view, visit
To unsubscribe, visit

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20ddbdd73a64fda3fe32fca98ee541aa4cead4b3
Gerrit-Change-Number: 9470
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <>
Gerrit-Reviewer: Adar Dembo <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Sat, 03 Mar 2018 00:06:56 +0000
Gerrit-HasComments: Yes

Reply via email to