Add option to allow empty array to match null

[lmartelli]

No description provided.

[ashleyfrieze]

Hey @lmartelli - this seems like a very specific condition. It's an unexpectedly specific one. I wonder whether we should implement this differently.

Can you give me some more context. What's the problem we're trying to solve here? We have an expected JSON that contains [] but you'd like to use [] to match null in the actual JSON? Why does the expected not just say null? Or are we saying that [] means [] or null in the actual?

Can we do this another way? Should we be saying that at(path, satisifies(isEmpty().or().isNull())) (pseudo code)?

[lmartelli]

Hi @ashleyfrieze

this seems like a very specific condition. It's an unexpectedly specific one. I wonder whether we should implement this differently.

Can you give me some more context. What's the problem we're trying to solve here? We have an expected JSON that contains [] but you'd like to use [] to match null in the actual JSON? Why does the expected not just say null? Or are we saying that [] means [] or null in the actual?

My context is verifying interaction with a service that happens to replace [] with null. Tested code should do a PUT on the service, and I check that in the test by doing a GET and asserting the result of that GET.

@Test
void can_add_exception() {
	Map<String, String> urlException = Map.of(
			"path", "/toto",
			"comment", "bla bla");
	assertThat(siteApi.urlExceptions().post(urlException))
			.isSuccess()
			.hasStatus(CREATED);

	verifyUrlExceptions(List.of(urlException));}

@Test
void can_delete_exception() {
	val urlExceptionId =
			siteApi.urlExceptions().post(
							Map.of("path", "/toto", "comment", "bla bla"),
							IdDto.class)
					.assertSuccess()
					.getBody()
					.getId();

	assertThat(siteApi.urlExceptions(urlExceptionId).delete())
			.hasStatus(NO_CONTENT);

	verifyUrlExceptions(emptyList());
}

private void verifyUrlExceptions(List<Map<String, String>> urlExceptions) {
	assertThat(siteApi.urlExceptions().getArray()).hasJsonBody(urlExceptions);
	if (urlExceptions.isEmpty())
		assertThatSiteIsProvisionedWith(assertJson -> assertJson.at("/urlsException").satisfies(new IsEmpty().or(NullCondition.getInstance())));
	else
		assertThatSiteIsProvisionedWith(Map.of("urlsException", map(urlExceptions, get("path"))));
}

private void assertThatSiteIsProvisionedWith(Object expected) {
	assertThat(shieldProvClient.getSiteConfig(siteName))
			.hasValueSatisfying(prov -> assertJson(prov).isEqualTo(expected));
}

private void assertThatSiteIsProvisionedWith(UnaryOperator<AssertJson<Object>> assertions) {
	assertThat(shieldProvClient.getSiteConfig(siteName))
			.hasValueSatisfying(prov -> assertions.apply(JsonAssertions.assertJson(prov)));
}

Needless to say, I have other fields to check.

With nullMatchesEmptyArray(), I could get rid of verifyUrlExceptions() and use the generic assertThatSiteIsProvisionedWith() directly :

assertThatSiteIsProvisionedWith(Map.of("urlsException", List.of(...))

Can we do this another way? Should we be saying that at(path, satisifies(isEmpty().or().isNull())) (pseudo code)?

This is my current workaround as you can see, but I was not able to write isEmpty().or().isNull(). Did I miss the static methods somewhere ?

[lmartelli]

I could probably convert [] to null in assertThatSiteIsProvisionedWith(). I'll try that ...

[lmartelli]

Not that easy :-(
Stream collectors do not accept null values in maps 😭

[lmartelli]

I now have a better solution in my tests, but still when the service will eventually return a proper [], the tests will break.
So it would still be useful for me to have nullMatchesEmptyArray().

[ashleyfrieze]

I think we could reasonably have an isNullOrEmpty condition to use with at, but having a something to soften isEqualTo via the where construct seems like it's going to encourage people to make their tests too fuzzy.

The isEqualTo is meant to be used when you can predict the exact outcome, but the where clause allows you to fuzzy some of the fields where you know their outcomes are unpredictable.

I think we can do more work to make the composing of conditions a bit easier too, since your current test code has to work hard to make the satisifies work.

I didn't have the static code I was talking about in my comment. I was just expressing in pseudocode the intention here.

What do you think is the highest priority useful change we can make from all the examples discussed so far?

[lmartelli]

Static methods to allow for more natural and concise conditions in satisfies looks like a good low hanging fruit to catch.
I can work on that.

[ashleyfrieze]

@lmartelli - if you want to do that, then be my guest. Maybe add some sort of Conditions class where these methods can appear in the bit of the package which is meant to be the public interface. The readme says that the internals aren't guaranteed for long-term compatibility, but that there are public entry points which are.

[ashleyfrieze]

@lmartelli - the trail's gone cold on this - what's your view on the way forward?