Abhishek Chennaka has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17989 )

Change subject: KUDU-1959 - Add tests for /startup page for the Master
......................................................................


Patch Set 5:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:

http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@390
PS4, Line 390:
> > I felt calling the function once with a longer string to match would be c
I see, makes sense. Agreed.


http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@394
PS4, Line 394: *\"read_data_directo
> Right after these you have .*, so this would match 1000 as well, so I still
Nice catch there. Will try to put in a "( |,)" to match the next character and 
handle that case. The only upside would be to get a strictly valid number as 
the status.


http://gerrit.cloudera.org:8080/#/c/17989/5/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:

http://gerrit.cloudera.org:8080/#/c/17989/5/src/kudu/master/master-test.cc@382
PS5, Line 382: const int wait_time = 10;
> nit: constant values are usually named per the GSG https://google.github.io
Makes sense, removing the use of const variable.


http://gerrit.cloudera.org:8080/#/c/17989/5/src/kudu/master/master-test.cc@407
PS5, Line 407:   run_status_reader = true;
> If the above two assertions fail, this line will be never reached and the t
Done


http://gerrit.cloudera.org:8080/#/c/17989/5/src/kudu/master/master-test.cc@407
PS5, Line 407:   run_status_reader = true;
> +1
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 09 Nov 2021 06:07:04 +0000
Gerrit-HasComments: Yes

Reply via email to