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

Fix failing tests in master #5410

Merged
merged 1 commit into from
Nov 12, 2019
Merged

Conversation

kenfinnigan
Copy link
Member

@kenfinnigan kenfinnigan commented Nov 12, 2019

I know the fix isn't ideal, as it adds sleep, but any alternative suggestions are appreciated

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

Let's get this in sooner rather than later

String CONTENT = "html content";
test.addResourceFile(META_INF_RESOURCES + "index2.html", CONTENT);

Thread.sleep(700);
Copy link
Member

Choose a reason for hiding this comment

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

wouldn't it be better to use Awaitibility and poll until it passes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Await on the RestAssured call passing?

And then have a max time of 2 seconds?

Is that preferred? I can adjust

Copy link
Member Author

Choose a reason for hiding this comment

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

We currently don't have Awaitility as a dependency, is it ok to add?

Copy link
Member

Choose a reason for hiding this comment

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

I have extensions using it I believe.

Copy link
Member Author

Choose a reason for hiding this comment

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

ugh, I was looking at the wrong BOM!

Copy link
Member Author

Choose a reason for hiding this comment

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

Tried switching to awaitility, and it has a failure with one, not entirely sure why

Copy link
Member Author

Choose a reason for hiding this comment

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

Awaitility seems to lead to just as much raciness unfortunately

@kenfinnigan kenfinnigan merged commit 2ebf84b into quarkusio:master Nov 12, 2019
@gsmet gsmet added this to the 1.1.0 milestone Nov 15, 2019
@kenfinnigan kenfinnigan deleted the fix-master branch November 18, 2019 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants