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

Jump to bottom

[Internal] Flaky Test: Fixes Client Telemetry Flaky tests #4760

Merged
merged 2 commits into from
Oct 8, 2024
Merged

[Internal] Flaky Test: Fixes Client Telemetry Flaky tests #4760

merged 2 commits into from
Oct 8, 2024
+47 −22

Conversation

sourabh1007
Copy link
Contributor

Description

Removing dependency on Task.Delay()

Type of change

  • [] Bug fix (non-breaking change which fixes an issue)

Closing issues

To automatically close an issue: closes #4731

@sourabh1007 sourabh1007 force-pushed the users/sourabhjain/clienttelemetryflakytest branch from 11eb639 to 1aaa004 Compare October 8, 2024 01:04
@sourabh1007 sourabh1007 marked this pull request as ready for review October 8, 2024 01:05
@sourabh1007 sourabh1007 self-assigned this Oct 8, 2024
philipthomas-MSFT
philipthomas-MSFT reviewed Oct 8, 2024
View reviewed changes
Copy link
Contributor

@philipthomas-MSFT philipthomas-MSFT left a comment

Choose a reason for hiding this comment

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

nit: more context on how this stabilizes the tests and are there things we need to avoid with future tests.

@kirankumarkolli kirankumarkolli merged commit 5aee50c into master Oct 8, 2024
23 checks passed
@kirankumarkolli kirankumarkolli deleted the users/sourabhjain/clienttelemetryflakytest branch October 8, 2024 16:39
@sourabh1007
Copy link
Contributor Author

nit: more context on how this stabilizes the tests and are there things we need to avoid with future tests.

Using Task.Delay(), we make assumptions about the duration of a process, but this may often fall short due to various factors. Instead of relying on Task.Delay(), it's better to adopt a signal-based approach, which ensures that the task we're waiting for has been completed, without being constrained by an estimated time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philipthomas-MSFT philipthomas-MSFT philipthomas-MSFT left review comments

@Pilchie Pilchie Pilchie approved these changes

@kirankumarkolli kirankumarkolli kirankumarkolli approved these changes

@khdang khdang Awaiting requested review from khdang khdang is a code owner

@sboshra sboshra Awaiting requested review from sboshra sboshra is a code owner

@adityasa adityasa Awaiting requested review from adityasa adityasa is a code owner

@neildsh neildsh Awaiting requested review from neildsh neildsh is a code owner

@FabianMeiswinkel FabianMeiswinkel Awaiting requested review from FabianMeiswinkel FabianMeiswinkel is a code owner

@kirillg kirillg Awaiting requested review from kirillg kirillg is a code owner

Assignees

@sourabh1007 sourabh1007

Labels
None yet
Projects
Azure Cosmos SDKs
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[ClientTelemetry] Client telemetry tests are flaky
4 participants
@sourabh1007 @Pilchie @kirankumarkolli @philipthomas-MSFT