Update MockCookie#parse(String) validation

[ngocnhan-tran1996]

spring-framework/spring-test/src/main/java/org/springframework/mock/web/MockCookie.java

Lines 130 to 132 in 68e6acd

	* Factory method that parses the value of the supplied "Set-Cookie" header.
	* @param setCookieHeader the "Set-Cookie" value; never {@code null} or empty
	* @return the created cookie

Based on the Javadoc, I think it should throw Set-Cookie header must not be null or empty instead of Invalid Set-Cookie header '' when setCookieHeader is empty.

[sbrannen]

Good catch! 👍

I've requested a minor change to the test.

Also, please update the duplicate variant of this class: spring-web/src/testFixtures/java/org/springframework/web/testfixture/servlet/MockCookie.java

Thanks

[sbrannen]

Suggested change

	@Test
	void parseEmptyHeader() {
	assertThatIllegalArgumentException()
	.isThrownBy(() -> MockCookie.parse(""))
	.withMessageContaining("Set-Cookie header must not be null or empty");
	assertThatIllegalArgumentException()
	.isThrownBy(() -> MockCookie.parse(" "))
	.withMessageContaining("Set-Cookie header must not be null or empty");
	@ParameterizedTest
	@ValueSource(strings = {"", " "})
	void parseEmptyHeader(String header) {
	assertThatIllegalArgumentException()
	.isThrownBy(() -> MockCookie.parse(header))
	.withMessageContaining("Set-Cookie header must not be null or empty");
	}

A @ParameterizedTest would be better here.

[sbrannen]

Thanks for making the requested changes.

Looks good now.

[sbrannen]

This has been merged into main in 8b89493 and slightly revised in 27a85be (since I realized after the merge that we can actually combine the "null" and "empty" tests into a single test method).

Thanks

[sbrannen]

Hi @ngocnhan-tran1996,

Due to a code freeze on main, I reverted those two commits via 951c1f3 and e78d3df.

After the 7.1 M1 release, I will cherry pick those two commits back on top of main for inclusion in 7.1 M2, retaining you as the author of your commit.

In light of that, can you please create a new issue (not a PR) with the title and body of this PR, referencing the two original commits (8b89493 and 27a85be) to be applied?

Or if you'd prefer that I do that, just let me know.

I apologize for the inconvenience.

Regards,

Sam

[ngocnhan-tran1996]

Hi @sbrannen

I created issue #37136. Please let me know if the content needs to be changed.

[sbrannen]

I created issue #37136. Please let me know if the content needs to be changed.

That looks perfect. Thanks, @ngocnhan-tran1996. 👍

I've now labeled this PR as:

• Superseded by Update MockCookie#parse(String) validation #37136

And I've already assigned that issue to the 7.1 M2 milestone.

Cheers,

Sam