search

eclipse-vertx / vertx-junit5

Testing Vert.x applications with JUnit 5
Apache License 2.0
42 stars 30 forks source link

vertx-junit5 should depend only on core #73

Closed slinkydeveloper closed 4 years ago

slinkydeveloper commented 4 years ago

In order to allow using vertx-junit5 for testing packages in vertx stack, vertx-junit5 (parent and childs) should depend only on vertx-core.

This issue blocks the release of vertx-web-validation, vertx-web-api-service and vertx-web-openapi, because it introduces circular dependencies: https://github.com/vert-x3/vertx-web/commit/690b6a4a9f06e891b4bc60cf2f13c3a0dd24761d

This issue is composed by:

  1. Removing vertx-rx-java and vertx-rx-java2 from vertx-junit5: https://github.com/vert-x3/vertx-junit5/blob/master/vertx-junit5/pom.xml#L22
  2. Removing vertx-web-client optional dependency from vertx-junit5: https://github.com/vert-x3/vertx-junit5/blame/0c62f06b8cf69d4d0b644136c10b282827028d6f/vertx-junit5/pom.xml#L30 (Is this dep just a mistake?)
  3. Removing vertx-junit5-web-client from vertx-junit5-parent compilation tree.
slinkydeveloper commented 4 years ago

My two cents:

  1. I propose two solutions:
    • Some ugly reflections to invoke Vertx rx java wrappers. Ugly, but not breaking.
    • Split this package to three different package, and move the rx ones to the respective rx projects (so vertx-rx-java2-junit5 under vertx-rx-java2 parent). This introduces a breaking change, since people will need to add a new dependency and use a new junit5 extension (like VertxRxExtension). Another solution, less breaking, could be to always have a single VertxExtension, which we extend through the ServiceLoader mechanism
  2. This fix should be trivial
  3. Assuming that we abandon the feature of the WebClient injection, we still need vertx-web-client types for the assertions. I don't want to "abandon" the dream of having focused assertions for vertx APIs, because it's a really useful feature for us and for users. Since other packages in future could join the assertions "party" the problem will raise again. But at this point I think It would be wise if single packages ships it, so web-client (maybe not with the production jar) should ship it.
vietj commented 4 years ago

can we have the vertx-junit5 extension for rx to be in vertx-rx project ?

vietj commented 4 years ago

ping

jponge commented 4 years ago

Let me think about it 🎶

jponge commented 4 years ago

I think moving the Rx-generated packages and modules to vertx-rx is the right thing to break circular dependencies.

Now the good question is how to keep VertxExtension the single point for the extension. Technically we could have a VertxRx2Extension, etc, but that wouldn't be so great.

So we need some pluggable mechanism for the types that can be injected. Thus we'd need a factory loaded through an SPI, right? 😉 The the VertxExtension would load these factories / types, and inject. What gets injected would then be what is on the class/module path.

jponge commented 4 years ago

...and in this case the webclient module is separate, but vertx-junit5 should not depend on it, it should just be a submodule in Maven parlance.

jponge commented 4 years ago

It's a non-trivial refactoring, basically it looks like we need to rewrite all VertxExtension internals around a flexible design to instanciate parameters to be injected.

I believe we need an SPI to discover "injectable types" (e.g., Vertx, WebClient, VertxTestContext, etc). The difficult points are that these objects need to be scoped (because you can have nested tests and such), they need to be closed (vertx.close()), and intialization requires parameters (e.g., WebClient needs to locate a matching Vertx instance, etc).

jponge commented 4 years ago

See #74 (work-in-progress)

jponge commented 4 years ago

All done 🎉