DefaultArgumentConverter no longer converts "null" to false

[JKomoroski]

Error message: Failed to convert String "null" to type java.lang.Boolean

Working Version: 5.9.2
Broken Version: 5.10.0

Steps to reproduce

Use the DefaultArgumentConverter to convert a string to a Boolean. Strings "true" and "false" convert correctly, but the string "null" no longer converts to a null Boolean (wrapper type) after upgrading.

Since this converter is used for @CsvSource data in @ParameterizedTest methods , it is quite common to include nulls in those sources for many types, not just Boolean.class.

A naive fix and test for this issue is included in my fork of junit5. I suspect a more robust fix for nullable types more broadly should be considered.

main...JKomoroski:junit5:main

Deliverables

• Fix behavior of nullable wrapper types to support "null" -> null conversions once again.

[sbrannen]

This is now the expected behavior due to #3177.

However, we may consider a way to switch back to lenient parsing of boolean values or make the true/false values somehow configurable.

[JKomoroski]

@sbrannen I'm not sure this title change accurately reflects the behavior change.

In the past "null" was being converted to null for nullable primate wrapper types (Boolean.class in this case). The new issue title indicates that the issue was expecting "null" -> false for a boolean primitive which is not the issue I'm having.

[sbrannen]

In the past "null" was being converted to null for nullable primate wrapper types (Boolean.class in this case).

My initial tests do not reveal that, but perhaps I am missing something.

Previously, we used Boolean::valueOf to convert a String to a Boolean, and that method returns false for "null". It also returns false for null.

The new issue title indicates that the issue was expecting "null" -> false for a boolean primitive which is not the issue I'm having.

If I'm not mistaken, we previously converted "null" to Boolean.FALSE or false for Boolean and boolean target types, respectively.

Based on that, this is technically a regression, but I don't yet know if we will "fix" this regression.

In any case, I'll introduce more extensive testing around this.

[sbrannen]

but the string "null" no longer converts to a null Boolean (wrapper type) after upgrading.

Were you perhaps using the nullValues = "null" feature of @CsvSource as in the following example?

@ParameterizedTest
@CsvSource(nullValues = "null", value = ...)
void myTest(...) {
}

If not, I don't see how "null" would have been converted to a Boolean null.

[sbrannen]

In summary, the following test passes against 5.9.x.

	@ParameterizedTest
	@CsvSource(nullValues = "null", textBlock = """
			apple,   True
			banana,  true
			lemon,   false
			kumquat, null
			""")
	void yummyFruit(String fruit, Boolean yummy) {
		switch (fruit) {
			case "apple" -> assertThat(yummy).isTrue();
			case "banana" -> assertThat(yummy).isTrue();
			case "lemon" -> assertThat(yummy).isFalse();
			case "kumquat" -> assertThat(yummy).isNull();
			default -> fail("Unexpected fruit : " + fruit);
		}
	}

Whereas, the following test fails against 5.9.x, since nullValues = "null" has been removed and yummy for kumquat is now false.

	@ParameterizedTest
	@CsvSource(textBlock = """
			apple,   True
			banana,  true
			lemon,   false
			kumquat, null
			""")
	void yummyFruit(String fruit, Boolean yummy) {
		switch (fruit) {
			case "apple" -> assertThat(yummy).isTrue();
			case "banana" -> assertThat(yummy).isTrue();
			case "lemon" -> assertThat(yummy).isFalse();
			case "kumquat" -> assertThat(yummy).isNull();
			default -> fail("Unexpected fruit : " + fruit);
		}
	}

To make the above test pass, you have to change the assertion for kumquat as follows.

	@ParameterizedTest
	@CsvSource(textBlock = """
			apple,   True
			banana,  true
			lemon,   false
			kumquat, null
			""")
	void yummyFruit(String fruit, Boolean yummy) {
		switch (fruit) {
			case "apple" -> assertThat(yummy).isTrue();
			case "banana" -> assertThat(yummy).isTrue();
			case "lemon" -> assertThat(yummy).isFalse();
			case "kumquat" -> assertThat(yummy).isFalse();
			default -> fail("Unexpected fruit : " + fruit);
		}
	}

The latter test passes against 5.9.x but fails against 5.10.x with the originally reported error message Failed to convert String "null" to type java.lang.Boolean.

So the regression is that "null" is no longer converted to a boolean false.

But... that makes sense, because we now only convert "false" (ignoring case) to a boolean false.

[sbrannen]

To observe the actual behavioral changes, consult the following commits.

• Status quo for 5.9.x: 738b13c
  • ported to 5.10.x and main unchanged: 849ce5e
• Status quo for 5.10.x and main: ab160b0

The latter commit makes it clear that the only thing that has changed is that we now only convert "false" (ignoring case) to a boolean false.

[JKomoroski]

This is very helpful. Thank you. I'll update my csv source tests with this in mind. I'll further investigate my other failing tests to see what else could be the problem.

[sbrannen]

This is very helpful. Thank you.

You're welcome.

I'll update my csv source tests with this in mind. I'll further investigate my other failing tests to see what else could be the problem.

OK. Let us know if you run into any other issues.

[JKomoroski]

All of my failures were related to @CsvSource changes. The other failures from the upgrade to 5.10 in my work do not have an underlying problem with Junit5. This issue can be closed.