-
Notifications
You must be signed in to change notification settings - Fork 35
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 test coverage for containerResponseFilter issue #1616
Conversation
run tests |
...anilla/src/test/java/io/quarkus/ts/http/restclient/reactive/VanillaReactiveRestClientIT.java
Outdated
Show resolved
Hide resolved
...active-vanilla/src/main/java/io/quarkus/ts/http/restclient/reactive/VersionHeaderFilter.java
Outdated
Show resolved
Hide resolved
...reactive-vanilla/src/main/java/io/quarkus/ts/http/restclient/reactive/RestCallerService.java
Outdated
Show resolved
Hide resolved
...anilla/src/test/java/io/quarkus/ts/http/restclient/reactive/VanillaReactiveRestClientIT.java
Outdated
Show resolved
Hide resolved
...anilla/src/test/java/io/quarkus/ts/http/restclient/reactive/VanillaReactiveRestClientIT.java
Outdated
Show resolved
Hide resolved
@mocenas I've tried it and the exception is not detectable, you were right about logging, let's stick with it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
AFAIC you can merge it or wait for @rsvoboda if he finds a time. Thanks
...active-vanilla/src/main/java/io/quarkus/ts/http/restclient/reactive/VersionHeaderFilter.java
Outdated
Show resolved
Hide resolved
...reactive-vanilla/src/main/java/io/quarkus/ts/http/restclient/reactive/RestCallerService.java
Outdated
Show resolved
Hide resolved
Remainder: Whoever merge this must use Squash and merge option. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I just realized you need to add note to README about new module. Then it's fine with me.
added |
* Add test coverage for containerResponseFilter issue Issue quarkusio/quarkus#31024
Summary
Adding test coverage for issue quarkusio/quarkus#31024.
For issue to manifest we need a quarkus app without resteasy dependency. It is probably cleaner and easier to maintain, to have a new module for this, that creating some system to exclude this dependency for some test. It would require a new build of quarkus app anyway, so this approach should not bring any more performance drawbacks.
Please select the relevant options.
run tests
phrase in comment)Checklist: