SoftAssertions Implementation #1340

uchagani · 2023-07-22T04:03:20Z

Feedback requested:

I took a stab at implementing soft assertions (issue #819). Before I continue, I'd love to get some feedback on if this is an acceptable approach.

I have implemented PageAssertions so far. Please take a look at TestSoftPageAssertions.java for what the usage would be. I used most of the existing test cases to exercise the method via soft assertions.

I believe this approach takes the least intrusive route (No existing playwright code was modified).

…ackage

yury-s · 2023-08-11T18:07:12Z

Hi @uchagani, sorry for long delay. Your approach looks sensible and we can accept your implementation of soft assertions into this repo.

A few notes about the implementation:

The API interfaces for Java (and all other language ports) are generated from the documentation sources that live upstream, e.g. PageAssertions. PageAssertions needs to be generated in a similar fashion. This way it will "automatically" appear on playwright.dev. For the latter, you may need to tweak format_java.js to make sure the documentation looks nice. Feel free to ask any questions as that code is quite convoluted.
All implementation classes should go under src/main/java/com/microsoft/playwright/impl.

Starting with PageAssertions is a good idea, let's worked out it first and then proceed with the locator assertions.

yury-s · 2023-08-11T18:09:11Z

playwright/src/main/java/com/microsoft/playwright/assertions/PageAssertionsImplProxy.java

+
+  @Override
+  public PageAssertions not() {
+    return new PageAssertionsImplProxy(super.results, (PageAssertionsImpl) pageAssertions.not());


Just change the return type of PageAssertionsImpl.not() to PageAssertionsImpl.

yury-s · 2023-08-11T18:11:38Z

playwright/src/main/java/com/microsoft/playwright/assertions/PageAssertionsProxy.java

+import java.util.List;
+import java.util.regex.Pattern;
+
+class PageAssertionsProxy extends SoftAssertionsBase implements PageAssertions {


Looks like this class is subsumed by PageAssertionsImplProxy and can be deleted.

yury-s · 2023-08-11T18:16:12Z

playwright/src/main/java/com/microsoft/playwright/assertions/SoftAssertionsImpl.java

+import java.util.List;
+
+class SoftAssertionsImpl implements SoftAssertions {
+  final List<Throwable> results;


I would use RuntimeException here and in PageAssertionsImplProxy and SoftAssertionsBase. We don't want to catch any Errors anyway and only catch unchecked exceptions in SoftAssertionsBase anyway.

@yury-s, we catch AssertionFailedError which is an Error

Wow, didn't know that. Then Throwable is justified.

uchagani · 2023-08-13T23:25:00Z

@yury-s I created a class-softassertions.md file but when i run roll I get the following error:

Error: Unexpected assertion class SoftAssertions
    at assertionArgument (/Users/umairchagani/dev/playwright.dev/src/generator.js:753:9)
    at JavaFormatter.formatMember (/Users/umairchagani/dev/playwright.dev/src/format_java.js:42:30)
    at Generator.createMemberLink (/Users/umairchagani/dev/playwright.dev/src/generator.js:506:27)
    at Generator.visitClassToc (/Users/umairchagani/dev/playwright.dev/src/generator.js:514:12)
    at Generator.generateClassDoc (/Users/umairchagani/dev/playwright.dev/src/generator.js:174:10)
    at new Generator (/Users/umairchagani/dev/playwright.dev/src/generator.js:140:12)
    at generateDocsForLanguages (/Users/umairchagani/dev/playwright.dev/src/generate.js:103:3)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async /Users/umairchagani/dev/playwright.dev/src/generate.js:156:5

How do i get it playwright to run with my new additions?

yury-s · 2023-08-14T23:31:54Z

@yury-s I created a class-softassertions.md file but when i run roll I get the following error:

Error: Unexpected assertion class SoftAssertions
    at assertionArgument (/Users/umairchagani/dev/playwright.dev/src/generator.js:753:9)
    at JavaFormatter.formatMember (/Users/umairchagani/dev/playwright.dev/src/format_java.js:42:30)
    at Generator.createMemberLink (/Users/umairchagani/dev/playwright.dev/src/generator.js:506:27)
    at Generator.visitClassToc (/Users/umairchagani/dev/playwright.dev/src/generator.js:514:12)
    at Generator.generateClassDoc (/Users/umairchagani/dev/playwright.dev/src/generator.js:174:10)
    at new Generator (/Users/umairchagani/dev/playwright.dev/src/generator.js:140:12)
    at generateDocsForLanguages (/Users/umairchagani/dev/playwright.dev/src/generate.js:103:3)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async /Users/umairchagani/dev/playwright.dev/src/generate.js:156:5

How do i get it playwright to run with my new additions?

Do you have the change uploaded somewhere? It might be that you need to update some of the generator scripts, I can try running it locally and help you.

Update: yeah, it looks like you need to update the generator around format_java.js:42 to support SoftAssertion class. Probably you can set empty prefix similar to if (member.clazz.varName === 'playwrightAssertions') { code a few lines above.

uchagani · 2023-08-14T23:42:16Z

@yury-s here is my branch with the markdown (https://github.com/uchagani/playwright/tree/java-soft-assertions). I'll take a look at your suggestion about changing format_java

uchagani · 2023-08-15T13:51:56Z

@yury-s so i added a || member.clazz.varName === 'softAssertions' to line 39 and now I can see that it created a class-softassertions.mdx file but i can't seem to see it when i run the dev server via npm run start-java is there something else I need to do?

yury-s · 2023-08-15T16:23:36Z

but i can't seem to see it when i run the dev server via npm run start-java is there something else I need to do?

I usually just follow the instructions and that's enough. Note that the new page will not show up in the sidebar automatically, you need to manually edit https://github.com/microsoft/playwright.dev/blob/main/java/sidebars.js, but you should be able to navigate to the page by typing its url. I can try to apply your patch locally later today if it still doesn't work.

uchagani · 2023-08-15T21:16:34Z

Ah ok I didn't know it wouldn't show up in the sidebar and I had to navigate to it. I'll try that in a bit.

uchagani · 2023-08-15T23:22:40Z

@yury-s so i can't navigate to it either. I think i'm doing something wrong because when I add it to the sidebar under assertions, none of the assertion sidebar items show up.

uchagani · 2023-08-15T23:28:41Z

Here is my playwright.dev branch: https://github.com/uchagani/playwright.dev/tree/java-soft-assertions

yury-s · 2023-08-16T21:07:20Z

@yury-s so i can't navigate to it either. I think i'm doing something wrong because when I add it to the sidebar under assertions, none of the assertion sidebar items show up.

By default it shows stable docs which don't contain the new page. You need to navigate to http://localhost:3000/java/docs/next/api/class-softassertions (note /next/ in the URL, it points to the dev version of the docs). You can press Shift 5 times to stick with the next docs on that page.

I ran your patches locally and I can see the page.

uchagani · 2023-08-16T22:33:18Z

that worked! thanks!

uchagani · 2023-08-17T00:24:52Z

@yury-s PRs created for both the main playwright repo and playwright.dev. Is there anything else I need to do before the new API interface will be available for this PR?

uchagani · 2023-08-17T03:02:48Z

@yury-s how does the api generator know to make a method static?

uchagani · 2023-08-17T13:55:55Z

https://github.com/microsoft/playwright-java/blob/main/tools/api-generator/src/main/java/com/microsoft/playwright/tools/ApiGenerator.java#L685 found it. i guess we'll have to add something here too to handle the static create method. I can add it once the other PRs are merged.

yury-s · 2023-08-17T17:05:54Z

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

yury-s · 2023-08-17T17:07:19Z

Oops, wrong PR, I meant the one upstream, sorry 😄

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

This reverts commit 632fba5.

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

Initial commit of SoftAssertions

2cc89cd

uchagani changed the title ~~Initial commit of SoftAssertions~~ SoftAssertions Implementation Jul 22, 2023

Create interface for SoftAssertions and move all code to assertions p…

d18c747

…ackage

uchagani marked this pull request as ready for review July 25, 2023 18:34

uchagani added 2 commits July 25, 2023 20:03

Fix some redundant methods, add @OverRide to methods

9f37c34

fix not() to return an instance of the proxy

d2751cb

yury-s reviewed Aug 11, 2023

View reviewed changes

uchagani mentioned this pull request Aug 17, 2023

docs(java): Add SoftAssertions for Java microsoft/playwright#26512

Merged

yury-s merged commit 632fba5 into microsoft:main Aug 17, 2023
16 of 19 checks passed

yury-s added a commit to yury-s/playwright-java that referenced this pull request Aug 17, 2023

Revert "SoftAssertions Implementation (microsoft#1340)"

aed4ce0

This reverts commit 632fba5.

yury-s added a commit that referenced this pull request Aug 17, 2023

Revert "SoftAssertions Implementation (#1340)" (#1357)

7d5953c

This reverts commit 632fba5.

uchagani mentioned this pull request Aug 23, 2023

feat(soft-assertions): Implement soft assertions for playwright-java #1361

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SoftAssertions Implementation #1340

SoftAssertions Implementation #1340

uchagani commented Jul 22, 2023 •

edited

Loading

yury-s commented Aug 11, 2023

yury-s Aug 11, 2023

yury-s Aug 11, 2023

yury-s Aug 11, 2023

uchagani Aug 19, 2023 •

edited

Loading

yury-s Aug 21, 2023

uchagani commented Aug 13, 2023

yury-s commented Aug 14, 2023 •

edited

Loading

uchagani commented Aug 14, 2023 •

edited

Loading

uchagani commented Aug 15, 2023 •

edited

Loading

yury-s commented Aug 15, 2023

uchagani commented Aug 15, 2023

uchagani commented Aug 15, 2023

uchagani commented Aug 15, 2023

yury-s commented Aug 16, 2023 •

edited

Loading

uchagani commented Aug 16, 2023

uchagani commented Aug 17, 2023

uchagani commented Aug 17, 2023

uchagani commented Aug 17, 2023

yury-s commented Aug 17, 2023

yury-s commented Aug 17, 2023

SoftAssertions Implementation #1340

SoftAssertions Implementation #1340

Conversation

uchagani commented Jul 22, 2023 • edited Loading

yury-s commented Aug 11, 2023

yury-s Aug 11, 2023

Choose a reason for hiding this comment

yury-s Aug 11, 2023

Choose a reason for hiding this comment

yury-s Aug 11, 2023

Choose a reason for hiding this comment

uchagani Aug 19, 2023 • edited Loading

Choose a reason for hiding this comment

yury-s Aug 21, 2023

Choose a reason for hiding this comment

uchagani commented Aug 13, 2023

yury-s commented Aug 14, 2023 • edited Loading

uchagani commented Aug 14, 2023 • edited Loading

uchagani commented Aug 15, 2023 • edited Loading

yury-s commented Aug 15, 2023

uchagani commented Aug 15, 2023

uchagani commented Aug 15, 2023

uchagani commented Aug 15, 2023

yury-s commented Aug 16, 2023 • edited Loading

uchagani commented Aug 16, 2023

uchagani commented Aug 17, 2023

uchagani commented Aug 17, 2023

uchagani commented Aug 17, 2023

yury-s commented Aug 17, 2023

yury-s commented Aug 17, 2023

uchagani commented Jul 22, 2023 •

edited

Loading

uchagani Aug 19, 2023 •

edited

Loading

yury-s commented Aug 14, 2023 •

edited

Loading

uchagani commented Aug 14, 2023 •

edited

Loading

uchagani commented Aug 15, 2023 •

edited

Loading

yury-s commented Aug 16, 2023 •

edited

Loading