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.

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

Ensure all metrics have enough time to be recorded #18041

Merged
merged 1 commit into from
Oct 5, 2020

Conversation

skabashnyuk
Copy link
Contributor

@skabashnyuk skabashnyuk commented Oct 5, 2020

What does this PR do?

Test was failing because this metric

    assertEquals(
        registry
            .get("executor.completed")
            .tag("name", MeteredExecutorServiceWrapperTest.class.getName())
            .tags(userTags)
            .functionCounter()
            .count(),
        1.0);

has not enough time to be recorded. With this pr all metrics have some timeout to be recorder ~1sec
Ensure all metrics have enough time to be recorder

Screenshot/screencast of this PR

What issues does this PR fix or reference?

fixes #18012

How to test this PR?

mvn clean install

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

Signed-off-by: Sergii Kabashniuk <skabashniuk@redhat.com>
@skabashnyuk skabashnyuk requested review from sparkoo, metlos and mshaposhnik and removed request for nickboldt and l0rd October 5, 2020 09:09
@skabashnyuk skabashnyuk changed the title Ensure all metrics have enough time to be recorder Ensure all metrics have enough time to be recorded Oct 5, 2020
@che-bot che-bot added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/bug Outline of a bug - must adhere to the bug report template. labels Oct 5, 2020
@che-bot
Copy link
Contributor

che-bot commented Oct 5, 2020

✅ E2E Happy path tests succeed 🎉

See Details

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

Copy link
Member

@sparkoo sparkoo left a comment

Choose a reason for hiding this comment

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

I've ran over 5000 iterations, passed 100%. It was failing rarely before.

@skabashnyuk skabashnyuk merged commit 2bd0ea2 into eclipse-che:master Oct 5, 2020
@skabashnyuk skabashnyuk deleted the che18012 branch October 5, 2020 20:16
@che-bot che-bot removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Oct 5, 2020
@che-bot che-bot added this to the 7.20 milestone Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Outline of a bug - must adhere to the bug report template.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

che observability unit test is failing randomly
5 participants