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

Improve camel-quarkus-bean-validator test coverage #3567 #3572

Merged
merged 1 commit into from Mar 2, 2022
Merged

Improve camel-quarkus-bean-validator test coverage #3567 #3572

merged 1 commit into from Mar 2, 2022

Conversation

JiriOndrusek
Copy link
Contributor

fixes #3567

Added tests showed the necessity of different configuration and/or registration for reflection. I added this information into the extension's documentation.

@JiriOndrusek JiriOndrusek force-pushed the 3567-improve-camel-quarkus-bean-validator-coverage branch from dca226c to 814f73d Compare February 23, 2022 12:46
@JiriOndrusek JiriOndrusek force-pushed the 3567-improve-camel-quarkus-bean-validator-coverage branch from 814f73d to de65e7a Compare February 24, 2022 07:31
jamesnetherton
jamesnetherton reviewed Feb 24, 2022
View reviewed changes
Copy link
Contributor

@jamesnetherton jamesnetherton left a comment

Choose a reason for hiding this comment

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

Looks ok to me. I left some documentation nitpick suggestions.

docs/modules/ROOT/pages/reference/extensions/bean-validator.adoc Outdated Show resolved Hide resolved
docs/modules/ROOT/pages/reference/extensions/bean-validator.adoc Outdated Show resolved Hide resolved
docs/modules/ROOT/pages/reference/extensions/bean-validator.adoc Outdated Show resolved Hide resolved
docs/modules/ROOT/pages/reference/extensions/bean-validator.adoc Outdated Show resolved Hide resolved
@JiriOndrusek JiriOndrusek force-pushed the 3567-improve-camel-quarkus-bean-validator-coverage branch 2 times, most recently from 67610a9 to 9535339 Compare February 28, 2022 11:05
@JiriOndrusek JiriOndrusek marked this pull request as draft February 28, 2022 12:33
@JiriOndrusek
Copy link
Contributor Author

@aldettinger thanks again for spotting the redundant code! It seems, that there was atypo which made xml test falsely positive. Moreover I fixed it and discovered, that xml validation does not work. I tried to find a reason and found a comment in jira ticket saying, that only annotations are supported in quarkus - see quarkusio/quarkus#5531 (comment)

Because there are no test in extension covering this feature. I think that it is still unsupported. I will change the tests and documentation accordingly. I'll also report an issue on qaurkus to make sure it is documented somewhere.

@JiriOndrusek JiriOndrusek force-pushed the 3567-improve-camel-quarkus-bean-validator-coverage branch from 9535339 to 1e9f1ed Compare March 1, 2022 10:02
@JiriOndrusek
Copy link
Contributor Author

JiriOndrusek commented Mar 1, 2022 •
edited

@aldettinger , @jamesnetherton I fixed problems with the test and made it simpler. I also add a new limitation into the documentation (xml configuration does not work - because of quarkus's limitation)

@JiriOndrusek JiriOndrusek marked this pull request as ready for review March 1, 2022 10:05
@jamesnetherton jamesnetherton merged commit 8681c76 into apache:main Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jamesnetherton jamesnetherton jamesnetherton approved these changes

@oscerd oscerd oscerd approved these changes

@zbendhiba zbendhiba zbendhiba approved these changes

@aldettinger aldettinger aldettinger 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.

Improve camel-quarkus-bean-validator test coverage
5 participants
@JiriOndrusek @jamesnetherton @oscerd @zbendhiba @aldettinger