Todd Lipcon 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)

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, w
yea, that's why I didn't change for now.. on the other hand it can be useful to 
know which sub-shard is actually flaky. I suppose longest term it woudl be nice 
to even track each test-case separately but that woudl require more mechanics


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.
Done


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.
Done


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
it seems like this 'name' actually never ends up displayed anywhere best I can 
tell. It probably corresponds to some ancient version of isolate. I'll just 
remove it.


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:
yea, though with looping a test you specify the whole command line yourself and 
typically filter to a single test. In other words, I almost always used the 
loop mode along with --disable-sharding. I couldn't figure out the best way to 
modify this for the new scheme. But you're right I also broke the existing 
functionality.



--
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-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Comment-Date: Sat, 03 Mar 2018 00:32:50 +0000
Gerrit-HasComments: Yes

Reply via email to