-
Notifications
You must be signed in to change notification settings - Fork 207
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
[Feature] Bring SoftAssertions to Java #819
Comments
That's certainly possible but the lack of good integrated test runner like the one in nodejs makes it less convenient in java, let's see if there is more interest in bringing this to java. |
If we could integrate it with assertJ’s soft assertions that might be good enough |
+1 This is definitely something we should see in java and c# |
+1, it should be brought to Java as well |
+1 |
Let's not forget about Python too! Please! |
Here is how to do it with JUnit 5: import org.junit.jupiter.api.Test;
import static com.microsoft.playwright.assertions.PlaywrightAssertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertAll;
public class TestSoftAssertions extends TestBase {
@Test
void testSoftAssertions() {
page.setContent("<div id=node>Text content</div>");
Locator locator = page.locator("text=content");
assertAll(
() -> assertThat(locator).hasAttribute("id", "force fail"),
() -> assertThat(locator).hasText("force fail")
);
}
} This will work with Playwright assertions which is awesome. |
@uchagani do you have a solution for TestNG :) |
I don't unfortunately. Personally I've moved all of my codebases to junit 5 because it's so much better than testng in terms of extensibility and functionality. |
+1, it's kind of blocking issue. |
+1 |
1 similar comment
+1 |
I've created a new package that brings soft assertions to Playwright-Java: https://github.com/uchagani/playwright-java-soft-assertions @yury-s @dgozman I have an open PR for this implementation to be merged into this repo if you guys like the approach. The full implementation is done and i can copy it over quickly but i'll need to update the tests to be more like how playwright does them which should only take a day or so. Please let me know because i believe this belongs in this repo instead of a separate package. |
We discussed the feature and its current implementation in the API review meeting today and several concerns were raised. We are going to revert the implementation for the upcoming release at least. Main concerns:
assertAll("Page assertions",
() -> assertThat(page).hasTitle("Playwright"),
() -> assertThat(page).hasURL("foo")
// PwPageSoftAssertions.java
package org.example.test;
import com.microsoft.playwright.Page;
import org.assertj.core.api.SoftAssertions;
public class PwPageSoftAssertions extends SoftAssertions {
public PwPageAssert assertThat(Page actual) {
return proxy(PwPageAssert.class, Page.class, actual);
}
}
// PwPageAssert.java
package org.example.test;
import com.microsoft.playwright.Page;
import com.microsoft.playwright.assertions.PageAssertions;
import org.assertj.core.api.AbstractAssert;
import java.util.regex.Pattern;
import static com.microsoft.playwright.assertions.PlaywrightAssertions.assertThat;
public class PwPageAssert extends AbstractAssert<PwPageAssert, Page> implements PageAssertions {
private final PageAssertions pageAssertions;
public PwPageAssert(Page page) {
super(page, PwPageAssert.class);
pageAssertions = assertThat(page);
}
@Override
public void hasTitle(String s, HasTitleOptions hasTitleOptions) {
pageAssertions.hasTitle(s, hasTitleOptions);
}
@Override
public void hasTitle(Pattern pattern, HasTitleOptions hasTitleOptions) {
pageAssertions.hasTitle(pattern, hasTitleOptions);
}
@Override
public void hasURL(String s, HasURLOptions hasURLOptions) {
pageAssertions.hasURL(s, hasURLOptions);
}
@Override
public void hasURL(Pattern pattern, HasURLOptions hasURLOptions) {
pageAssertions.hasURL(pattern, hasURLOptions);
}
@Override
public PageAssertions not() {
return pageAssertions.not();
}
}
// Test method
@Test
public void testAssertjPageSoftAssertions() {
Playwright playwright = Playwright.create();
Browser browser = playwright.chromium().launch();
BrowserContext context = browser.newContext();
Page page = context.newPage();
page.navigate("https://playwright.dev");
PwPageSoftAssertions softly = new PwPageSoftAssertions();
softly.assertThat(page).hasTitle("Playwright");
softly.assertThat(page).hasURL("foo");
softly.assertThat(2).isEqualTo(1);
softly.assertAll();
} Current implementation of SoftAssertions does not help in this, moreover it encourages users to create a separate instance instance of SoftAssertions. We decided to hold off this feature until these concers are addressed. |
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.
The motivation behind #1361 was to enable calling playwright-java's auto-waiting assertions in a soft way, without tying it to any other library, similar to playwright-java's assertions themselves. They are not assertj assertions or testng assertions.
While JUnit's soft assertions work, it is awkward. You would basically have to wrap the whole test in an
This was never the intention of #1361 because then we are assuming that the user is using TestNG. We can certainly provide better integration with TestNG similar to what we're doing with JUnit in #1371 but I think these are two different things: soft assertions for playwright vs soft assertions for playwright for TestNG.
I think this is the same issue as tying soft assertions to another library. Since playwright-java's assertions are not tied to AssertJ I don't see why the calling these same assertions in a soft way has to be. If we tie it to AssertJ now we are limiting users to AssertJ and not other matchers that they might be using just because they want to call them softly.
This is correct but i'm open to other suggestions on how to handle this without tying it to a particular test runner (JUnit/TestNG) or a particular matcher (AssertJ/Hamcrest/). playwright-java's assertions are self contained so that's what #1361 does. We can provide support for AssertJ, Hamcrest, JUnit, and TestNG separately just for soft assertions but that seems like an odd thing to do when playwright-java's assertions don't care which test runner or assertion library one is using. |
I understand the objective was to provide soft assertions in a manner that does not tie it to any other libraries. There is a difference, though, between playwright web assertions and soft assertions on top of them. Playwright assertions introduced features not available before (auto-waiting). On the other hand, while adding soft assertions is definitely a convenience, similar functionalities could be implemented within existing testing frameworks using playwright assertions. Different testing frameworks take different approaches to soft assertions and if we'd like to be idiomatic, we need to play by their rules and provide a more seamless integrations. This is one of the reasons we were hesitant bringing this feature to java right away. We should have spotted this earlier. Since there is no clear-cut solution that works for all frameworks, we usually hold-off shipping any suboptimal implementations and this what I suggest we do here. We can finish JUnit integration and then can see how this can fit into the new strucutre. Does it make sense?
Even though it may be cumbersome, this is how JUnit recommends to do soft assertions and changing their framework is not our goal. From our experience, it's usually sufficient to have a block of multiple soft asserts followed by actions, followed by another block of soft asserts. One rarely needs to check soft assertions scattered across distant parts of the test body. In those scenarions
That was not a goal indeed. During the initial code review I was under impression that there is no built-in support in the current frameworks and everyone does it their own way and having SoftAsserts/assertAll was one of the best practices. On a closer look, it turned out that the client will likely have to reimplement SoftAsserts if they want good integration with existing builtin soft asserts. I should have spotted this problem earlier. This is our regular practice in Playwright to no ship a feature (especially in language ports) if there is controversy.
I'd focus on JUnit integration and then get back to this. I believe it'd be better if we provide a solid integration with one of the testing frameworks, e.g. AssertJ, than if we end up with an implementation that doesn't integrate well with any framework.
I tried to explain above the difference from our perspective, basically soft assertions can be implemented using builtin mechanisms in each of these frameworks and playwright assertions APIs. |
…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.
Node version has soft assertions now. Can we get soft assertions for Java too?
The text was updated successfully, but these errors were encountered: