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

Fix Quarkus-cli flacky test #321

Merged
merged 1 commit into from
Oct 29, 2021
Merged

Fix Quarkus-cli flacky test #321

merged 1 commit into from
Oct 29, 2021

Conversation

pjgg
Copy link
Contributor

@pjgg pjgg commented Oct 28, 2021 •
edited
Loading

@pjgg pjgg requested a review from Sgitario October 28, 2021 10:27
@pjgg pjgg force-pushed the fix/run_conditions branch from 685cf0a to 819de5e Compare October 28, 2021 10:28
Sgitario
Sgitario suggested changes Oct 28, 2021
View reviewed changes
Copy link
Contributor

@Sgitario Sgitario left a comment

Choose a reason for hiding this comment

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

This sounds odd to me. The app.start() should be consistent and if's flaky, it should be fixed internally.

What is happening here is that for some reason, it fails to start at first and then it retries several times until eventually it starts ok.

@pjgg pjgg force-pushed the fix/run_conditions branch from 819de5e to d91f325 Compare October 28, 2021 12:25
@pjgg pjgg requested a review from Sgitario October 28, 2021 12:26
Sgitario
Sgitario suggested changes Oct 28, 2021
View reviewed changes
Copy link
Contributor

@Sgitario Sgitario left a comment

Choose a reason for hiding this comment

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

This fix seems like a workaround for CLI but that now affects all the scenarios in the test framework with possible side effects.
We should investigate why this test starts behaving flacky to indeed solve the issue.

@pjgg pjgg force-pushed the fix/run_conditions branch from d91f325 to 7ab82f1 Compare October 28, 2021 14:09
@pjgg pjgg requested a review from Sgitario October 28, 2021 14:10
This was referenced Oct 28, 2021
Closed
Closed
Sgitario
Sgitario reviewed Oct 29, 2021
View reviewed changes
Copy link
Contributor

@Sgitario Sgitario left a comment

Choose a reason for hiding this comment

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

I'm ok with the changes, but add a comment with a link to the upstream issue / or pr to put some context around why this was necessary.

@pjgg pjgg force-pushed the fix/run_conditions branch from 7ab82f1 to f50b0ae Compare October 29, 2021 07:29
@pjgg pjgg requested a review from Sgitario October 29, 2021 07:31
@pjgg pjgg force-pushed the fix/run_conditions branch from f50b0ae to 135aa05 Compare October 29, 2021 07:40
@pjgg pjgg requested a review from Sgitario October 29, 2021 07:40
@Sgitario Sgitario merged commit 9126dcb into quarkus-qe:main Oct 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Sgitario Sgitario Sgitario approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pjgg @Sgitario