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

Add tests for #543 #581

Merged
merged 1 commit into from Jan 2, 2020
Merged

Add tests for #543 #581

merged 1 commit into from Jan 2, 2020

Conversation

lburgazzoli
Copy link
Contributor

No description provided.

@lburgazzoli
Copy link
Contributor Author

ok to test

Copy link
Contributor

@ppalaga ppalaga left a comment

Choose a reason for hiding this comment

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

The test per se looks good, but I do not understand how is this supposed to work on the Quarkus Platform? I see you disabled the new test on the platform, but I suspect the test container won't even start due to the missing my-routes.xml file? Can't we make the my-routes.xml a classpath resource and keep the new test enabled on the platform?

@lburgazzoli
Copy link
Contributor Author

We can but then we need to include the routes in the native image which is not yet done automatically, in general we should not assume that all the test have to run on the quarkus-platform

@ppalaga
Copy link
Contributor

ppalaga commented Jan 2, 2020

We can but then we need to include the routes in the native image which is not yet done automatically

I'd vote for including the XML file in the native image via application.properties' for now:

quarkus.native.additional-build-args = -H:IncludeResources=my-routes.xml

Not sure this will work OOTB on the platform, but it is a bit more flexible than -H:ResourceConfigurationFiles recommended in the Quarkus docs.

in general we should not assume that all the test have to run on the quarkus-platform

I think we should assume that all our tests should run there unless we define a clear filtering strategy.

@lburgazzoli
Copy link
Contributor Author

lburgazzoli commented Jan 2, 2020

Peter, we already agreed that we need to create some consolidated test for the quarkus platform.

@lburgazzoli
Copy link
Contributor Author

xml is now included in the native image

@lburgazzoli
Copy link
Contributor Author

ok to test

Copy link
Contributor

@ppalaga ppalaga left a comment

Choose a reason for hiding this comment

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

Perfect, thanks!

@oscerd oscerd merged commit 8fd7a41 into apache:master Jan 2, 2020
@lburgazzoli lburgazzoli deleted the github-543 branch January 3, 2020 07:52
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.

None yet

3 participants