Assert return value of callable throwing exception

[t-8ch]

Summary

It would be nice if the assertThatCode().doesNotThrowException() API would be able execute functions that return a value and then return that value for further inspection.
This would make it easy to provide a nice description to an otherwise maybe nondescript exception that is thrown.

Example

@Test 
public void intendedApi() {
  int x = assertThatCode(this::doStuff)
    .describedAs("doing things")
    .doesNotThrowAnyException();
  assertThat(x).isEqualTo(3);
}

@Test
public void currentSolution1() {
  int x;
  try {
    x = doStuff();
  } catch (Exception e) {
    fail("doing things");
    return; // never reached; needed because fail() is not recognized by the java compiler as method exit
  }
  assertThat(x).isEqualTo(3);
}

@Test
public void currentSolution2() {
  AtomicReference<Integer> x = new AtomicReference<>();
  assertThatCode(() -> x.set(doStuff()))
    .describedAs("doing things")
    .doesNotThrowAnyException();
  assertThat(x.get()).isEqualTo(3);
}

// utility for demonstration purposes
public int doStuff() throws Exception {
  if (new Random().nextBoolean()) {
    throw new Exception();
  }
  return 3;
}

[onacit]

How about...

assertThatCode(() -> ...)
        .doesNotThrowAnyExceptionAndReturnsSatisfying(Consumer<? super T> consumer)
;
assertThatCode(() -> ...)
        .doesNotThrowAnyExceptionAndReturnsSatisfying(IntConsumer consumer)
;

?

[joel-costigliola]

sorry this one fell under our radar, we'll look at it for 3.22.0.
my preference goes for returning object assertion after doesNotThrowAnyException() if technically possible

[joel-costigliola]

I don't think it is technically possible in the current codebase to change assertThatCode to support the suggested pattern.
assertThatCode returns an AbstractThrowableAssert and catches any exception from the given the lambda, to support the suggested pattern, assertThatCode would have to return an Object assertion for the values returned by the given lambda code (or null if void) and the caught exception, I feel this is too much of a breaking change.

assertThatCode was added a few years ago, its naming is a bit misleading as you don't always expect a piece of code to throw an exception, in that regard assertThatThrownBy is a better pattern as it's clear we expect the given code to throw an exception thus returning AbstractThrowableAssert makes complete sense.

At the moment IMO the best approach would be to let the test fail with the exception:

@Test
public void otherSolution() {
  int x = doStuff();
  assertThat(x).isEqualTo(3);
}

I know the test won't fail with an assertion error but with the exception from doStuff, I think that's an acceptable compromise and that's the option with the simplest test code.

I'm still thinking we could something like though:

// chains Object assertion
assertThatCodeReturnedValue(() -> doStuff()).isEqualTo(3);

// use asInstanceOf to chain specific typed assertion:
assertThatCodeReturnedValue(() -> doStuff()).asInstanceOf(INTEGER).isLessThan(10);

WDYT?

[joel-costigliola]

Better naming:

// chains Object assertion
assertThatValueReturnedBy(() -> doStuff()).isEqualTo(3);

// use asInstanceOf to chain specific typed assertion:
assertThatValueReturnedBy(() -> doStuff()).asInstanceOf(INTEGER).isLessThan(10);

[scordio]

I think it would be possible to add an overloaded version of assertThatCode() accepting Callable and exposing a result() method, so we would be able to write something like:

assertThatCode(() -> "asd".isEmpty()) // executes Callable::call storing either the result or the thrown exception
  .doesNotThrowAnyException()
  .result() // returns AbstractObjectAssert
  .isEqualTo(42);

still keeping compatibility with everything else which is not a Callable:

assertThatCode(() -> System.out.println("")) // AbstractThrowableAssert
  .doesNotThrowAnyException();

I'll prepare a draft PR for evaluation.

[onacit]

Hi @scordio. Is this in progress? Thanks.

[scordio]

Hi @onacit, yes, see #2519.