Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Introduce soft assertions for WebTestClient #26969

Closed
wants to merge 1 commit into from
Closed

Introduce soft assertions for WebTestClient #26969

wants to merge 1 commit into from

Conversation

wyhasany
Copy link
Contributor

@wyhasany wyhasany commented May 23, 2021 •
edited by sbrannen

Overview

It happens very often that WebTestClient is used in heavyweight integration tests. It's no use to waste time to check if another condition has been fixed or not. Soft assertions could help a lot to check all conditions at once even if one of them fails.

Proposal

New API would look like as follows:

client.get().uri("/test")
	.exchange()
	.expectAllSoftly(
		exchange -> exchange.expectStatus().isOk(),
		exchange -> exchange.expectBody(String.class)
			.isEqualTo("It works!")
	);

Related Issues

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 23, 2021
@sbrannen sbrannen added in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels May 23, 2021
@sbrannen sbrannen added this to the 5.3.8 milestone May 23, 2021
@sbrannen sbrannen self-assigned this May 23, 2021
@sbrannen sbrannen changed the title Soft assertions for WebTestClient Introduce soft assertions for WebTestClient May 23, 2021
@wyhasany wyhasany force-pushed the feature/assert-softly-for-web-test-client branch from b97efa4 to 38c6105 Compare May 24, 2021 09:56
@wyhasany
Soft assertions for WebTestClient
0b2c503 
It happens very often that WebTestClient is used in
heavyweight integration tests. It's no use to waste
time to check if another condition has been fixed or
not. Soft assertion could help a lot to check all
conditions at once even if one of them fail.

New API would look like as follows:
```java
client.get().uri("/test")
	.exchange()
	.expectAllSoftly(
		exchange -> exchange.expectStatus().isOk(),
		exchange -> exchange.expectBody(String.class)
			.isEqualTo("It works!")
	);
```
@wyhasany wyhasany force-pushed the feature/assert-softly-for-web-test-client branch from 38c6105 to 0b2c503 Compare May 24, 2021 10:28
@sbrannen sbrannen modified the milestones: 5.3.8, 5.3.9 Jun 7, 2021
@sbrannen sbrannen modified the milestones: 5.3.9, 5.3.10 Jul 5, 2021
@sbrannen sbrannen added the status: blocked An issue that's blocked on an external project change label Aug 22, 2021
@sbrannen
Copy link
Member

This PR is blocked until #26917 has been merged into main.

@sbrannen sbrannen mentioned this pull request Aug 23, 2021
Closed
@sbrannen sbrannen removed the status: blocked An issue that's blocked on an external project change label Aug 23, 2021
sbrannen pushed a commit to sbrannen/spring-framework that referenced this pull request Aug 23, 2021
@wyhasany @sbrannen
Introduce soft assertions for WebTestClient
d3c39c0 
It happens very often that WebTestClient is used in heavyweight
integration tests, and it's a hindrance to developer productivity to
fix one failed assertion after another. Soft assertions help a lot by
checking all conditions at once even if one of them fails.

This commit introduces a new expectAllSoftly(..) method in
WebTestClient to address this issue.

client.get().uri("/hello")
	.exchange()
	.expectAllSoftly(
		spec -> spec.expectStatus().isOk(),
		spec -> spec.expectBody(String.class).isEqualTo("Hello, World")
	);

Closes spring-projectsgh-26969
sbrannen added a commit to sbrannen/spring-framework that referenced this pull request Aug 23, 2021
@sbrannen
Polish soft assertion support for WebTestClient
3f5eb77 
See spring-projectsgh-26969
@sbrannen sbrannen closed this in 25dca40 Aug 23, 2021
sbrannen added a commit that referenced this pull request Aug 23, 2021
@sbrannen
Polish soft assertion support for WebTestClient
3c2dfeb 
See gh-26969
@sbrannen
Copy link
Member

sbrannen commented Aug 23, 2021 •
edited

This has been merged into main in 25dca40 and revised in 3c2dfeb.

Thanks for the 2nd soft assertions PR! 👍

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Sep 3, 2021 •
edited by sbrannen

Sorry for the late review. Overall looks good, but I've spotted a follow-up issue to be fixed.

Currently individual expectations like responseSpec.expectStatus() are internally surrounded with ExchangeResult#assertWithDiagnostics which logs request and response details. That means when multiple assertions are executed, those details are also logged multiple times.

I'm not immediately sure how to solve this, but we'll need to find something.

@rstoyanchev rstoyanchev reopened this Sep 3, 2021
@sbrannen
Copy link
Member

sbrannen commented Sep 3, 2021 •
edited

Currently individual expectations like responseSpec.expectStatus() are internally surrounded with ExchangeResult#assertWithDiagnostics which logs request and response details. That means when multiple assertions are executed, those details are also logged multiple times.

Good catch.

I'm not immediately sure how to solve this, but we'll need to find something.

OK. Let's brainstorm.

Maybe we could use a ThreadLocal to track whether that information has already been logged for the current thread.

@sync-by-unito sync-by-unito bot mentioned this pull request Sep 15, 2021
Closed
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this pull request Mar 26, 2022
@wyhasany @lxbzmy
Introduce soft assertions for WebTestClient
bae7e03 
It happens very often that WebTestClient is used in heavyweight
integration tests, and it's a hindrance to developer productivity to
fix one failed assertion after another. Soft assertions help a lot by
checking all conditions at once even if one of them fails.

This commit introduces a new expectAllSoftly(..) method in
WebTestClient to address this issue.

client.get().uri("/hello")
	.exchange()
	.expectAllSoftly(
		spec -> spec.expectStatus().isOk(),
		spec -> spec.expectBody(String.class).isEqualTo("Hello, World")
	);

Closes spring-projectsgh-26969
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this pull request Mar 26, 2022
@sbrannen @lxbzmy
Polish soft assertion support for WebTestClient
5ed53c7 
See spring-projectsgh-26969
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this pull request Mar 26, 2022
@rstoyanchev @lxbzmy
Ensure one time logging for request details
3e53ce9 
Closes spring-projectsgh-26969
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cuspymd cuspymd cuspymd left review comments

@kkocel kkocel kkocel approved these changes

Assignees

@sbrannen sbrannen

Labels
in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Milestone
5.3.10
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@wyhasany @sbrannen @rstoyanchev @kkocel @cuspymd @spring-projects-issues