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

Run tests on Netty for WebFlux #112

Open
malafeev opened this issue Jul 17, 2019 · 10 comments
Open

Run tests on Netty for WebFlux #112

malafeev opened this issue Jul 17, 2019 · 10 comments

Comments

@malafeev
Copy link

WebFlux doesn't require jetty or tomcat.
It uses netty.
Is it possible to get rid of jetty from WebFlux integration tests?

@geoand
Copy link
Collaborator

geoand commented Jul 17, 2019

Is this causing problems for you for some reason?

@malafeev
Copy link
Author

@geoand
Copy link
Collaborator

geoand commented Jul 17, 2019

So I might be missing something here... Is this tied to another issue perhaps?

@malafeev
Copy link
Author

If I use in my app tomcat then I see that TracingSubscriber.onComplete() is called.
But if I don't use tomcat and rely on netty which comes with spring-webflux I don't see calling TracingSubscriber.onComplete().
That's why I'm curious why integration test for WebFlux is based on jetty.
I use the latest version of spring.

@geoand
Copy link
Collaborator

geoand commented Jul 17, 2019

@csabakos can you shed some light on this perhaps since you were the one that added those tests?

Thanks

@csabakos
Copy link
Contributor

@geoand WebFlux should work equally over any runtime (Tomcat, Jetty, Netty, etc.), so there was no reason to switch away from the runtime that was already used in other tests.

If the observed behavior is different on Netty, then I think it would make sense to add Netty-based integration tests to make sure that all cases are covered.

@geoand
Copy link
Collaborator

geoand commented Jul 17, 2019

So you just added like that for convenience, thanks that's good to know.

@malafeev Would you like to take a stab at it? If you are unavailable, I can take a look but it will probably be a while...

@malafeev
Copy link
Author

@geoand it would be great if you take a look even in a while. Currently I don't have enough time.

@geoand
Copy link
Collaborator

geoand commented Jul 17, 2019

Same here :).

I'll take a look as soon as I can

@geoand
Copy link
Collaborator

geoand commented Aug 22, 2019

I want to upgrade to Opentracing 0.33 first. Then I'll look into this

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

No branches or pull requests

3 participants