Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19679 )
Change subject: [Python] Support setting startup flags in tests ...................................................................... Patch Set 4: (6 comments) http://gerrit.cloudera.org:8080/#/c/19679/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19679/4//COMMIT_MSG@7 PS4, Line 7: setting startup flags adding extra flags ? http://gerrit.cloudera.org:8080/#/c/19679/4//COMMIT_MSG@28 PS4, Line 28: chrony chronyd http://gerrit.cloudera.org:8080/#/c/19679/4/python/kudu/tests/common.py File python/kudu/tests/common.py: http://gerrit.cloudera.org:8080/#/c/19679/4/python/kudu/tests/common.py@35 PS4, Line 35: master services masters http://gerrit.cloudera.org:8080/#/c/19679/4/python/kudu/tests/common.py@49 PS4, Line 49: tserver services either 'tservers' or 'tablet servers' http://gerrit.cloudera.org:8080/#/c/19679/4/python/kudu/tests/common.py@130 PS4, Line 130: extra_tserver_flags Typo? Should probably be decorator_extra_tserver_flags? http://gerrit.cloudera.org:8080/#/c/19679/4/python/kudu/tests/test_common.py File python/kudu/tests/test_common.py: http://gerrit.cloudera.org:8080/#/c/19679/4/python/kudu/tests/test_common.py@56 PS4, Line 56: @master_extra_startup_flags(extra_master_flags) : @tserver_extra_startup_flags(extra_tserver_flags) : class TestKuduTestBaseStartupFlagsPositive(KuduTestBase, CompatUnitTest): : : def setUp(cls): : pass : : def test_startup_flags_positive(self): : pass It seems this test doesn't cover the newly added functionality: there is a typo at https://gerrit.cloudera.org/#/c/19679/4/python/kudu/tests/common.py@130 and this test didn't catch that. I'd think that a proper test would check the corresponding processes have been actually started with the set of provided flags, no? -- To view, visit http://gerrit.cloudera.org:8080/19679 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2423b29707e58d152aff934908fb3f0400b75da4 Gerrit-Change-Number: 19679 Gerrit-PatchSet: 4 Gerrit-Owner: Marton Greber <greber...@gmail.com> Gerrit-Reviewer: Alexey Serbin <ale...@apache.org> Gerrit-Reviewer: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Zoltan Chovan <zcho...@cloudera.com> Gerrit-Comment-Date: Tue, 04 Apr 2023 05:57:06 +0000 Gerrit-HasComments: Yes
