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

Allow Playwright in Runtime module and GraalVM support #95

Merged
merged 6 commits into from
Oct 28, 2024
Merged

Conversation

melloware
Copy link
Contributor

@melloware melloware commented Oct 25, 2024

This will allow using Playwright at Runtime for people who want to screen scrape.

  • Allow use at runtime for people who want to use it for screen scraping or other purposes
  • rename playwright to playwright-test
  • Fix Playwright in Quarkus Native (not for E2E testing) #68 Native mode
  • Replace System.out.println logging with Jboss logging with Graal Substitution

NOTE: this required me to rename the artifactId for playwright to playwright-test which is a breaking change so this should be a 2.0.0 if we go forward.

image

@melloware melloware force-pushed the runtime branch 21 times, most recently from 2b4d5a5 to 50e39a4 Compare October 25, 2024 20:48
@melloware melloware force-pushed the runtime branch 2 times, most recently from baadb02 to cd3ccc5 Compare October 26, 2024 17:08
@melloware melloware marked this pull request as ready for review October 26, 2024 17:08
@melloware melloware requested a review from a team as a code owner October 26, 2024 17:08
@melloware melloware added this to the 2.0.0 milestone Oct 26, 2024
@melloware melloware added the enhancement New feature or request label Oct 26, 2024
@melloware melloware changed the title Allow Playwright in Runtime module and GrallVM support Allow Playwright in Runtime module and GraalVM support Oct 27, 2024
@maxandersen
Copy link
Member

sounds like good idea to me. whats the migration path? remove -test but keep it in import scope test?

Any reason to not also provide a artifact named -test that just includes the non-test one ?

@melloware
Copy link
Contributor Author

That is a good question. You are saying I could theoretically put all the test stuff in the runtime module and poeple could choose whether or not to mark it <scope>test</scope> or not?

@ia3andy
Copy link
Contributor

ia3andy commented Oct 28, 2024

That is a good question. You are saying I could theoretically put all the test stuff in the runtime module and poeple could choose whether or not to mark it <scope>test</scope> or not?

I also thought about it :) we should try

@melloware
Copy link
Contributor Author

let me try it!

@maxandersen
Copy link
Member

yes and no - my main question was what the migration path was (if any) and if that was to just remove "-test" then why not just have "-test" include the new runtime one and all is good...

@melloware
Copy link
Contributor Author

Oh sorry the migration path would be if you use this now.

<dependency>
    <groupId>io.quarkiverse.playwright</groupId>
    <artifactId>quarkus-playwright</artifactId>
    <version>${playwright.version}</version>
    <scope>test</scope>
</dependency>

it changes to

<dependency>
    <groupId>io.quarkiverse.playwright</groupId>
    <artifactId>quarkus-playwright-test</artifactId>
    <version>${playwright.version}</version>
    <scope>test</scope>
</dependency>

@melloware
Copy link
Contributor Author

OK it looks like it works! I moved the Test code into the Runtime library and just marked junit5 as an OPTIONAL dependency.

@ia3andy
Copy link
Contributor

ia3andy commented Oct 28, 2024

I think that's ok as the testing part is really small and won't be in the target user app. @aloubyansky any thoughts?

@aloubyansky
Copy link
Member

You mean the optional dependency on the quarkus-junit5-internal?

@ia3andy
Copy link
Contributor

ia3andy commented Oct 28, 2024

Yes

@melloware
Copy link
Contributor Author

Yep

 <dependency>
            <groupId>io.quarkus</groupId>
            <artifactId>quarkus-junit5</artifactId>
            <optional>true</optional>
        </dependency>

@aloubyansky
Copy link
Member

What specifically do you achieve with that?

@melloware
Copy link
Contributor Author

Because we offer Unit testing classes with @WithPlaywright that are QuarkusTest only but also allow PlayWright to be used as a ScreenScraper. So anyone using it as a Test library will do this...

<dependency>
    <groupId>io.quarkiverse.playwright</groupId>
    <artifactId>quarkus-playwright</artifactId>
    <version>${playwright.version}</version>
    <scope>test</scope>
</dependency>
<dependency>
     <groupId>io.quarkus</groupId>
      <artifactId>quarkus-junit5</artifactId>
       <scope>test</scope>
</dependency>

and anyone wanting to use it as a runtime screen scraper can do this....

<dependency>
    <groupId>io.quarkiverse.playwright</groupId>
    <artifactId>quarkus-playwright</artifactId>
    <version>${playwright.version}</version>
</dependency>

@aloubyansky
Copy link
Member

Ah, not the internal one and the user has to still add the dependency on the quarkus-junit5 - sure, makes sense. Thanks.

@melloware
Copy link
Contributor Author

I was also able to Graal substitute as Playwright uses System.out.println for logging with Jboss logging!

@melloware melloware merged commit 6b6331d into main Oct 28, 2024
1 check passed
@melloware melloware deleted the runtime branch October 28, 2024 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Playwright in Quarkus Native (not for E2E testing)
4 participants