eclipse-vertx / vertx-junit5

Testing Vert.x applications with JUnit 5
Apache License 2.0
44 stars 32 forks source link

Remove callback hell while testing with futures #52

Closed slinkydeveloper closed 6 years ago

slinkydeveloper commented 6 years ago

Hi, While playing with async assertion I found that actual assertions api can cause a callback hell like this:

    persistance.addEvent(event).setHandler(test.succeeding(pushedEvent ->
        persistance.getEvent(pushedEvent.getId()).setHandler(test.succeeding(retrievedEvent -> {
          // Some assertions
          test.completeNow();
        }))
    ));

In this example I have only two async calls to simplify the example, but in some cases I have 4/5 calls that must be done sequentially :smile: I want to reuse the cool futures api to avoid a callback hell. I tried to create two local methods:

  private <T> Function<Throwable, Future<T>> assertNotFail(VertxTestContext test) {
    return res -> {
      test.failNow(res);
      return Future.failedFuture(res);
    };
  }

  private Future<Void> completeNow(VertxTestContext test) {
    test.completeNow();
    return Future.succeededFuture();
  }

This two methods could be included directly in VertxTestContext. With this methods I refactored the previous code like this:

    persistance
        .addEvent(event)
        .recover(this.assertNotFail(test)) // Check if failed
        .compose(pushedEvent -> persistance.getEvent(pushedEvent.getId())) // Run next async task
        .recover(this.assertNotFail(test)) // Check if failed
        .compose(retrievedEvent -> { 
          // Some assertions
         return this.completeNow(test); // Complete test and return completed future
        });

WDYT? There are other smartest ways to avoid callback hells while testing?

jponge commented 6 years ago

Hey @slinkydeveloper, fancy a pull request? :-)

vietj commented 6 years ago

I would prefer not, in Vert.x 4.0 we are moving toward CompletableFuture as we would have to deprecate that in 3.6 and remove it in 4.0

jponge commented 6 years ago

I would like to have this until 4.0 becomes more tangible, and for sure having this in 3.6 makes sense.

People do use futures as a concurrent model, and since we are talking about few lines of code it’ll make their life better over the next 6 months, possibly more since futures are not exactly disappearing.

We can revisit deprecation at the first 4.0 milestone.

Le 18 oct. 2018 à 05:20, Julien Viet notifications@github.com a écrit :

I would prefer not, in Vert.x 4.0 we are moving toward CompletableFuture as we would have to deprecate that in 3.6 and remove it in 4.0

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

slinkydeveloper commented 6 years ago

The smartest way to solve this problem is to have a method in vertx futures like Scala andThen (https://github.com/eclipse-vertx/vert.x/pull/2343) that handles in a mapper both a failed and a completed future.

Without adding nothing new to vertx futures, we can simply create two methods in VertxTestContext like:

// returns completed future if complete, fails test if fut fail and returns failed future
<T> Future<T> assertComplete(Future<T> fut);
// returns failed future if fails, fails test if fut complete and returns failed future
<T> Future<T> assertFailure(Future<T> fut);

With this api, a futs chain should be like:

assertComplete(fut1.compose(fut2).compose(fut3).recover(fut4))
  .setHandler(someAssertions);

or

assertFailure(fut1.compose(fut2).compose(fut3).recover(fut4));

And if you want to check step by step the future chain, that's the result

assertComplete(fut1)
  .compose(res -> assertComplete(fut2))

WDYT @jponge about this api?

jponge commented 6 years ago

Merged.