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.

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

docs(java): Add SoftAssertions for Java #26512

Merged
merged 1 commit into from
Aug 17, 2023

Conversation

uchagani
Copy link
Contributor

@uchagani uchagani commented Aug 17, 2023

This PR adds the SoftAssertions API for Java. Related PR: microsoft/playwright-java#1340

Note: microsoft/playwright.dev#1135 needs to be merged in order for the markdown in this PR to be rendered without errors

cc: @yury-s

}
```

## method: SoftAssertions.create
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, you'll need to update ApiGenerator when rolling this change to have all static methods generated in the API.

* since: v1.38
* langs: java

Runs all the assertions have been executed for this [SoftAssertions] object. If any assertions fail, this method throws an AssertionFailedError with the details of all the failed assertions.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Runs all the assertions have been executed for this [SoftAssertions] object. If any assertions fail, this method throws an AssertionFailedError with the details of all the failed assertions.
Runs after all the assertions have been executed for this [SoftAssertions] object. If any assertions failed, this method throws an AssertionFailedError with the details of all the failed assertions.

SoftAssertions softly = SoftAssertions.create();
```

## method: SoftAssertions.expectLocator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with adding expectLocator and expectAPIResponse right away, but we'll have to stub them as unimplemented for now during the roll. Alternatively, you could add them later to the docs, up to you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the implementation of these already worked out but i haven't committed it yet. If it's okay with you, i think we can do everything together.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good.

@yury-s
Copy link
Member

yury-s commented Aug 17, 2023

Once this lands, let's roll new driver with the new API as a separate PR keeping all new methods unimplemented and then apply your implementation on top of it in a separate PR.

@yury-s
Copy link
Member

yury-s commented Aug 17, 2023

I'll commit this right-away to unblock you. Feel free to address any comments in a follow-up PR.

@yury-s yury-s merged commit 475c96d into microsoft:main Aug 17, 2023
@yury-s
Copy link
Member

yury-s commented Aug 17, 2023

FYI, this run should include your change in the driver.

yury-s added a commit to yury-s/playwright that referenced this pull request Sep 6, 2023
yury-s added a commit that referenced this pull request Sep 6, 2023
As explained in
microsoft/playwright-java#819 (comment)
we are not ready to ship this in its current form. Reverting for now.

This reverts commit 475c96d.
Germandrummer92 pushed a commit to OctoMind-dev/playwright that referenced this pull request Oct 27, 2023
This PR adds the `SoftAssertions` API for Java. Related PR:
microsoft/playwright-java#1340

Note: microsoft/playwright.dev#1135 needs to be
merged in order for the markdown in this PR to be rendered without
errors
Germandrummer92 pushed a commit to OctoMind-dev/playwright that referenced this pull request Oct 27, 2023
…icrosoft#26917)

As explained in
microsoft/playwright-java#819 (comment)
we are not ready to ship this in its current form. Reverting for now.

This reverts commit 475c96d.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants