Throw AssumptionViolatedException when single null is passed to Assum…

[xendo]

…e.assumeNotNull(Object... objects) method

Previous implementation threw NullPointerException when single null argument was passed:
java.lang.NullPointerException
at java.util.Objects.requireNonNull(Objects.java:203)
at java.util.Arrays$ArrayList.(Arrays.java:3813)
at java.util.Arrays.asList(Arrays.java:3800)

[kcooney]

Thanks!

[kcooney]

BTW, are you absolutely sure the test would fail without the code changes? I would think you would need to pass in a null array to trigger the new code path

[xendo]

Kevin,

Out of those two:

    @Test
    public void arraysNull() throws Exception
    {
        Arrays.asList(null);
    }

    @Test
    public void arraysArrayNull() throws Exception
    {
        Object[] objects = {null};
        Arrays.asList(objects);
    }

only the first one fails with:

java.lang.NullPointerException
at java.util.Objects.requireNonNull(Objects.java:203)
at java.util.Arrays$ArrayList.(Arrays.java:3813)
at java.util.Arrays.asList(Arrays.java:3800)
at com.amazon.personid.service.test.integration.ArraysTest.arraysNull

[xendo]

Also it's fair to say that this invocation produces warning:

Warning:(14, 23) java: non-varargs call of varargs method with inexact argument type for last parameter;
cast to java.lang.Object for a varargs call
cast to java.lang.Object[] for a non-varargs call and to suppress this warning

More details here: http://stackoverflow.com/a/4028086

Based on that stackoverflow thread, I'm tempted to say that the current behavior is fine. It should be rare to explicitly pass in null here

[kcooney]

To extrapolate, the following is the only way to get a NPE out of this method:

assumeNotNull(null);

and

Object[] values = null;
assumeNotNull(values);

The first isn't clearly a valid use of this API. The second should be exceedingly rare.

[xendo]

Kevin,

You are correct. This is much more subtle than I originally thought. If I was using Java I would have never caught this. Reproducing this in Groovy is, however, much simpler:

    def "assumeNotNull should throw AssumptionViolatedException when passed single null value" () {
        given:
        String string = null
        when:
        assumeNotNull(string)
        then:
        thrown AssumptionViolatedException
    }

FYI in Java invocation on any type of Array will throw e.g.

String[] values = null;
assumeNotNull(values);

[marcphilipp]

Thanks! I've added a two inline comments requesting changes.

[marcphilipp]

Please add a cast to Object[] here.

[marcphilipp]

Please update the Javadoc:

If called with a {@code null} array or one or more {@code null} elements in {@code objects},
the test will halt and be ignored.

[stefanbirkner]

Thanks! Could you please update the release notes?