Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deflaked: Guarddog_impl_test #14475

Merged
merged 3 commits into from
Dec 22, 2020
Merged

Deflaked: Guarddog_impl_test #14475

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7bb6af0
Deflaked guarddog_impl_test; on overloaded machines it would sometime…
KBaichoo Dec 17, 2020
cac3b9d
Do extrapetting only under the real time system.
KBaichoo Dec 21, 2020
dbfd111
Merge remote-tracking branch 'upstream/master' into deflake-guarddog-…
KBaichoo Dec 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions test/server/guarddog_impl_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,8 +624,14 @@ TEST_P(GuardDogActionsTest, MissShouldOnlyReportRelevantThreads) {
// This will reset the loop interval timer, and should help us
// synchronize with the guard dog.
guard_dog_->forceCheckForTest();
// Touch the second_dog in case we overslept in the real time system
// and the guard dog timer goes off.
second_dog_->touch();

time_system_->advanceTimeWait(std::chrono::milliseconds(51));
// Touch the second_dog in case we overslept in the real time system
// and the prior "touch" was consumed.
second_dog_->touch();
guard_dog_->forceCheckForTest();

EXPECT_THAT(events_, ElementsAre("MISS : 10"));
Expand Down Expand Up @@ -686,8 +692,14 @@ TEST_P(GuardDogActionsTest, MegaMissShouldOnlyReportRelevantThreads) {
// This will reset the loop interval timer, and should help us
// synchronize with the guard dog.
guard_dog_->forceCheckForTest();
// Touch the second_dog in case we overslept in the real time system
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we do this extra petting only when this test is run in real-time mode?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do that if you really want, but the extra petting it shouldn't affect the simulated time test much as we're really just trying to get one of the watch dogs petted and have the other not petted over the given threshold, so that when we do the final check the latter one triggers the event.

Petting the former more often doesn't change that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I suggest that is that part of the goal of the simulated time test is that it should be completely deterministic in its behavior regardless of how loaded the testing machine is.

I think this workaround that's specific to real-time means we don't gain the confidence that the simtime version is 100% deterministic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// and the guard dog timer goes off.
second_dog_->touch();

time_system_->advanceTimeWait(std::chrono::milliseconds(51));
// Touch the second_dog in case we overslept in the real time system
// and the prior "touch" was consumed.
second_dog_->touch();
guard_dog_->forceCheckForTest();

EXPECT_THAT(events_, ElementsAre("MEGAMISS : 10"));
Expand Down