Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14379 )

Change subject: WIP [tests] use system_unsync time source for tests by default
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/14379/1/src/kudu/clock/hybrid_clock-test.cc
File src/kudu/clock/hybrid_clock-test.cc:

http://gerrit.cloudera.org:8080/#/c/14379/1/src/kudu/clock/hybrid_clock-test.cc@355
PS1, Line 355:   FLAGS_time_source = "system";
             :   clock_.reset(new HybridClock);
             :   ASSERT_OK(clock_->Init());
> This is in anticipation of making 'builtin' the default value?
Nope: this test scenario implicitly assumed it's run against 'system' time 
source.  It expects ntp-related diagnostics to be output.  Once the default 
time source is switched from 'system' (in this patch it's 'system_unsync'), 
it's necessary to explicitly turn enable the 'system' time source for this 
scenario.


http://gerrit.cloudera.org:8080/#/c/14379/1/src/kudu/util/test_util.cc
File src/kudu/util/test_util.cc:

http://gerrit.cloudera.org:8080/#/c/14379/1/src/kudu/util/test_util.cc@133
PS1, Line 133:   FLAGS_time_source = UseSystemNtp() ? "system" : 
"system_unsync";
> Sorta weird to do this in KuduTest (and thus incur the odd "up the tree" de
I tried IsGTest() to set the default value for time_source, and it works if 
getting rid of probing for the env variable (because the code for the latter 
lives in kudu_test_util as of now).

However, I thought we might want to allow overriding the time source for our 
tests via environment variable or alike.  At least that could allow running the 
whole bunch of tests using the specified time source.

Or we don't need that at all?



--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Oct 2019 02:16:34 +0000
Gerrit-HasComments: Yes

Reply via email to