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 #476 List itests in an XML file for the Quarkus platform #482

Merged
merged 1 commit into from Nov 28, 2019

Conversation

ppalaga
Copy link
Contributor

@ppalaga ppalaga commented Nov 28, 2019

There is no version in the XML list of tests anymore as proposed by @lburgazzoli.
The XML is now deployed directly instead of being deployed in a jar.

@lburgazzoli
Copy link
Contributor

wonder if as future evolution, the xml with the list of the integration-test could be auto generated like all the it with a specific property are included (or excluded)

@ppalaga
Copy link
Contributor Author

ppalaga commented Nov 28, 2019

bd419b9 fixes the license header problem

@ppalaga
Copy link
Contributor Author

ppalaga commented Nov 28, 2019

be auto generated like all the it with a specific property are included (or excluded)

(Not sure you noticed) it is currently autogenerated and there is some file path based filtering there. Is that not enough? https://github.com/apache/camel-quarkus/pull/482/files#diff-e0dff2635520bef5f237973025e0a295R51-R63

@lburgazzoli
Copy link
Contributor

oh, damn me. didn't notice

@lburgazzoli
Copy link
Contributor

got confused because I saw the generated file also included in the repo, maybe as it is auto generated it should not be checked in

@ppalaga
Copy link
Contributor Author

ppalaga commented Nov 28, 2019

auto generated it should not be checked in

I prefer to have it in git to be able to check anytime quickly what we export and also to have the history thereof. WDYT?

Now fixing the JAXB issue on Java 11.

@lburgazzoli
Copy link
Contributor

in general I'd go for not including auto generated code as It may be another source of conflicts and confusion as people may try to change them manually but it is not a huge issue

@ppalaga
Copy link
Contributor Author

ppalaga commented Nov 28, 2019

Yes, potential merge conflicts in the XML file would be nice to avoid.

The main motivation for me to keep the file in Git for now is that we have some irregularities in the integration-tests directory and the selection done by the plugin may get broken in the future. Having a chance to spot malign changes during the PR review could prevent that. - That's actually a sign that we should try to find a mechanism for exporting the tests that would be more robust than the current path matching. I vote for merging this PR as is and continue the discussion withing #413 . Either the selection could work based on properties in the test POMs (the irregularities in main an core are still there) as you proposed or maybe we could put the aggregated tests into a new directory where we'd keep a strictly flat layout (so that the path matching would be less likely to break).

@lburgazzoli
Copy link
Contributor

ok to test

@lburgazzoli
Copy link
Contributor

@ppalaga I have no objection to keep it now but long term solution would be better to remove it.

We may think to move support test extension in a dedicated support folder so main and core and future it tests are just like other tests and we only need to skip indexing the support folder

@ppalaga
Copy link
Contributor Author

ppalaga commented Nov 28, 2019

move support test extension in a dedicated support folder so main and core and future it tests are just like other tests

What an elegant option!

@asf-ci
Copy link

asf-ci commented Nov 28, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/camel-quarkus-pr/439/

@lburgazzoli lburgazzoli merged commit 0eac61c into apache:master Nov 28, 2019
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