hamcrest / JavaHamcrest

Java (and original) version of Hamcrest
http://hamcrest.org/
BSD 3-Clause "New" or "Revised" License
2.11k stars 378 forks source link

shortcut functionality of org.hamcrest.core.AnyOf no longer seems to work #335

Closed fvanwesterop closed 3 years ago

fvanwesterop commented 3 years ago

When upgrading from Hamcrest 1.3 to 2.2 in a client project I noticed some tests now failed. It seems to be because the shortcut capability of org.hamcrest.core.AnyOf no longer works, actualy since version 2.1 already.

The Javadoc claims that:

... Evaluation is shortcut, so subsequent matchers are not called if an earlier matcher returns true.

But that no longer seems to hold: Now when the earlier matcher returns true, the subsequent matchers are called regardless.

The test below illustrates this: The emptyOrNullString() matcher will return true so that the containsString() matcher is not expected to be called. It is though, breaking the test with a java.lang.IllegalArgumentException: missing substring. With hamcrest-core 1.3 this test still passes but since hamcrest 2.1 it will break.

package org.hamcrest.sample;

import org.junit.Test;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.*;

public class AnyOfTest {

    @Test
    public void testAnyOf() {

        // given:
        String someNullableValue = "";
        String someNullableSubstring = null;

        // expect:
        assertThat(someNullableValue,
                anyOf(emptyOrNullString(), containsString(someNullableSubstring))
        );

    }

}
fvanwesterop commented 3 years ago

Things are not what they seem ;-) It is actually the changed implementation of the constructor of org.hamcrest.core.SubstringMatcher that causes the perceived behaviour:

In version 2.1 and later it is now doing a null-check on the substring we might want to match against later on. Since the matcher constructors are always executed (regardless wheter anyOf() would call the matcher's matches() method later on), an exception is now thrown. It turns out the second matcher actually would have been shortcut, if only the constructor would have omitted the nullcheck, as it did previously in version 1.3.

So no bug, it just means that it is no longer possible to assert in this way that some String value is either null, empty or contains a certain substring.