Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9470 )
Change subject: build: enable sharding within cmake/ctest ...................................................................... Patch Set 2: (5 comments) Makes sense, I'm much happier to see the sharding definitions colocated with test definitions. http://gerrit.cloudera.org:8080/#/c/9470/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9470/2//COMMIT_MSG@26 PS2, Line 26: * Flaky-test tracking is currently still done by the test binary name : and not the specific shard, though we could easily switch that in : the future. This gets weird though because the number of shards may change over time, which would muck up the sharded test's history. http://gerrit.cloudera.org:8080/#/c/9470/2/CMakeLists.txt File CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/9470/2/CMakeLists.txt@678 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 in dist_test.py. Update. http://gerrit.cloudera.org:8080/#/c/9470/2/build-support/dist_test.py File build-support/dist_test.py: http://gerrit.cloudera.org:8080/#/c/9470/2/build-support/dist_test.py@56 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. http://gerrit.cloudera.org:8080/#/c/9470/2/build-support/dist_test.py@322 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 cosmetic? http://gerrit.cloudera.org:8080/#/c/9470/2/build-support/dist_test.py@442 PS2, Line 442: execution = dict(argv=(["run-test.sh", 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 http://gerrit.cloudera.org:8080/9470 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I20ddbdd73a64fda3fe32fca98ee541aa4cead4b3 Gerrit-Change-Number: 9470 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Sat, 03 Mar 2018 00:06:56 +0000 Gerrit-HasComments: Yes
