Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22482 )
Change subject: IMPALA-13751: Fix runtime-profile-test failure since IMPALA-13304 ...................................................................... Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/22482/3/be/src/util/runtime-profile-test.cc File be/src/util/runtime-profile-test.cc: http://gerrit.cloudera.org:8080/#/c/22482/3/be/src/util/runtime-profile-test.cc@2079 PS3, Line 2079: // avg : > Actually, this was the reason to have missing_event_instances_expected. Then add such assertion in this method. EXPECT_TRUE(expect_complete_event || !missing_event_instances_expected.empty()); No test should be skipped. SetUp and Validate must assert separately. Having expect_complete_event parameter brings clarity to developer who read your test code. http://gerrit.cloudera.org:8080/#/c/22482/5/be/src/util/runtime-profile-test.cc File be/src/util/runtime-profile-test.cc: http://gerrit.cloudera.org:8080/#/c/22482/5/be/src/util/runtime-profile-test.cc@2273 PS5, Line 2273: 0.5f > This was the reason we had mutiple SetUp and TearDowns. Please make SetUp method return boolean saying whether it produce missing event scenario or not. If it not producing missing event scenario, repeat the SetUp until it does. That is what I mean by deterministic scenario. -- To view, visit http://gerrit.cloudera.org:8080/22482 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If04744e215cac79f255b3d73c3e91e873c13749a Gerrit-Change-Number: 22482 Gerrit-PatchSet: 6 Gerrit-Owner: Surya Hebbar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Laszlo Gaal <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Surya Hebbar <[email protected]> Gerrit-Comment-Date: Wed, 19 Feb 2025 19:00:01 +0000 Gerrit-HasComments: Yes
