karatelabs / karate

Test Automation Made Simple
https://karatelabs.github.io/karate
MIT License
8.1k stars 1.94k forks source link

explore adding first-class support for playwright #2291

Closed ptrthomas closed 1 month ago

ptrthomas commented 1 year ago

for context - karate has support for Playwright via the (somewhat undocumented) wire-protocol, and we were (one of) the first to use that approach (sep 2020): https://twitter.com/ptrthomas/status/1307678474627244032

but it is experimental, and no one stepped forward to help improve it. some teams tried it - but it does have limitations / bugs: https://stackoverflow.com/q/68490053/143475

since then playwright has put a lot of effort into an official java library: https://github.com/microsoft/playwright-java - it is being very actively developed

plan here is to see if we can come out with karate-playwright

f-delahaye commented 12 months ago

Is anyone looking at this? If not, i'll try to give it a go. I'm no Playwright expert, but their api does look close to what Karate supports. Locators/selectors and a few other things might be tricky but we'll see.

Just a few questions:

Thanks

ptrthomas commented 12 months ago

@f-delahaye you can go ahead. don't worry about the existing driver, we can throw it away if needed. I would prefer that playwright support is added as a new karate-playwright dependency, but don't let that stop you with starting off with what you find comfortable in a PR. yes to all your other points. there is a test-suite here: https://github.com/karatelabs/karate/tree/master/karate-e2e-tests

f-delahaye commented 11 months ago

Thanks @ptrthomas. I have a preliminary version that passes the tests, but needs some rework / clean up.

It aims at using the Playwright apis as much as possible, especially Locators but that raises a few points where your feedback is welcome:

Sorry, that's a lot of questions!

ptrthomas commented 11 months ago

@f-delahaye sounds great !

f-delahaye commented 11 months ago

Thanks!

My bad, the 30ms thing is probably a bit misleading. waitUntil('#mydiv', "function() {console.log(new Date()); return myEvaluationFunction") prints a new line every 30ms but that's in the browser. Things might be different on the java side.

Anyway ... my version now supports:

Note that the new PlaywrightDriver will take precedence over the "legacy" one if karate-playwright is added as a dependency in the pom.xml before karate-core.

Testing-wise, LocalPlaywrightRunner passes all the tests except 04.feature (for reason explained above) so it has now been skipped for driver 'playwright', and some lines of 13.feature (i suspect Wikipedia have actually changed their layout, driver 'chrome' fails there too) which have now been commented out.

I have changed 00.feature to re-enable some of the tests for PW, and also to add 18.feature and 19.feature. I'm not sure if it was a good idea. 19.feature for example fails with LocalSingleRunner at locate('//i').rightOf().find('//span') but that does not look like something Karate supports so maybe that particular test is not relevant...

I didn't run any tests other than LocalPlaywrightRunner (and LocalSingleRunner). Not sure if my home pc can run all the docker stuff...

Is that enough to create the PR? Or else, what are the next steps?

ptrthomas commented 11 months ago

@f-delahaye I think you can just go ahead with a PR. karate 1.5.X will change the maven groupId to io.karatelabs and I've just merged the jdk17 branch to develop so watch out for that. if you have localplaywrightrunner or singlerunner working, that is good ! I can look at the docker side later

ptrthomas commented 11 months ago

this has been merged ! thanks @f-delahaye

this may need some documentation and testing before release, there will certainly be 1.5.0.RC2 in a week or two. having regression tests using the docker container that embeds chrome will be good to have

anildhiman88 commented 9 months ago

Hi @ptrthomas When you are planning to release this fix in karate-core?

ptrthomas commented 9 months ago

@anildhiman88 this is available right now in Karate 1.5.0.RC2 (just released) - note that the artifact grouo-id has changed to io.karatelabs

f-delahaye commented 9 months ago

Thanks @ptrthomas

Agree about updating the documentation. I believe the main points are in the javadoc. Will try to create a README out of it and improve it a bit.

Definitely agree about testing. I had an opportunity to use this code in a real life (very very simple) case and while it worked, I'm not fully comfortable yet. Aside from bugs, one of my concerns is the implementation of delay/retry already mentioned in a previous comment. So, basically, feedback is welcome. And if delay/retry does not look right, please reach out to me.

ptrthomas commented 9 months ago

posted on social media

LinkedIn: https://www.linkedin.com/feed/update/urn:li:activity:7137691423725780992 Twitter: https://twitter.com/getkarate/status/1731926887595913593

azope25 commented 9 months ago

Hi @f-delahaye / @ptrthomas , When I was testing karate-playwright on my local for my tests, getting below "Timeout exceeded" error as by default set to 100ms.

org.graalvm.polyglot.PolyglotException: Error {
  message='Timeout 100ms exceeded.

 logs 
waiting for locator("xpath=//div[contains(normalize-space(text()),'xyz')]").first()
  locator resolved to <div class="more ng-binding"> xyz </div>
attempting click action
  waiting for element to be visible, enabled and stable
  element is visible, enabled and stable
  scrolling into view if needed
  done scrolling
  performing click action

  name='TimeoutError
  stack='TimeoutError: Timeout 100ms exceeded.

Also getting below error while using Driver Java api in my feature file.

My java code:- if(driver.exists("{button}ReadMe")){  ........

Error
org.graalvm.polyglot.PolyglotException: Error {
  message='Unsupported token "{" while parsing selector "{button}ReadMe"
  name='Error
  stack='Error: Unsupported token "{" while parsing selector "{button}ReadMe"

Can you please help me how to set timeout? Already configure karate retry in config file

Thanks

f-delahaye commented 9 months ago

@azope25 Thanks for the report! I will look at fixing the second issue. For the first one, would you be able to use retry as a workaround? Also, any chance you could share the scenario causing the timeout so i can investigate further?

azope25 commented 9 months ago

@f-delahaye , I already configured karate.configure('retry',{ count:10, interval:3000}); in my karate-config.js file. Is anything need to do extra here?

I can not share my scenario due to limitation but I will try to create some dummy project and try to reproduce issue and share

I think for slow loading application is causing the issue as timeout is only 100ms but not sure

Thanks

f-delahaye commented 9 months ago

I think for slow loading application is causing the issue as timeout is only 100ms but not sure

@azope25 I don't disagree but assuming the timeout happened on a action, the doc states "By default, all actions such as click() will not be re-tried - and this is what you would stick to most of the time, for tests that run smoothly and quickly", so I assume no delay in addition to no retry. But I probably got that wrong. Maybe @ptrthomas can clarify this and what the "default" interval should be, if any ... 3s?
Anyway, there is definitely a bug here because karate.configure('retry',{ count:10, interval:3000}) should have forced a delay. Will look at it as well. In the meantime, * retry(10, 3000).click(...) might help (although it's not a global setting)

azope25 commented 9 months ago

In the meantime, * retry(10, 3000).click(...) might help (although it's not a global setting)

@f-delahaye This workaround is not working for me.

js failed:
>>>>
01: retry(3,3000).waitFor(icons.search_icon).click()
<<<<
org.graalvm.polyglot.PolyglotException: Error {
  message='Timeout 100ms exceeded.
f-delahaye commented 9 months ago

My understanding is that:

azope25 commented 9 months ago

@f-delahaye Thank you for detail explanation. I will try at my end

azope25 commented 9 months ago

@f-delahaye My Observations regarding below points are:-

Looking at the documentation and other Drivers implementations, global retry settings do not apply to actions. So, this is not a bug, as far as I can tell.

As per my knowledge, current action methods(e.g. click,clear etc) behavior in Karate is it wait for global retry setting defined in karate.config file like karate.configure('retry',{ count:10, interval:3000}) and override global retry if we explicitly use retry along with action methods. By looking into code written for playwright, Can we use resolveLocator().click(new ClickOptions().setTimeout(driver.waitTimeout())); instead of resolveLocator().click(new ClickOptions().setTimeout(driver.actionWaitTimeout()));. I tested this code with global retry setting and explicit use of retry function and its working as expected as karate methods works with other driver implementation.

Also observed that playwright throw exception (strict mode violation ) if more than one locator found but in karate it returns first locator by default. To fix this, Can we use like Locator resolveLocator(PlaywrightToken token) { return token.toLocator().first(); }

Please correct me if I misunderstood.

f-delahaye commented 9 months ago

Upon further testing, it looks like global retryCount is not used, but global retryInterval is! i.e.the other drivers try the action, wait retryInterval ms, try again and fail. So I will change actionWaitTimeout() accordingly. Good point, @azope25, thanks!

Indeed, resolveLocator is designed (among other things) to avoid the strict mode violation. I thought it was called internally in all cases, did you find some where it is not?

azope25 commented 9 months ago

Indeed, resolveLocator is designed (among other things) to avoid the strict mode violation. I thought it was called internally in all cases, did you find some where it is not?

@f-delahaye I think ofRoot(locator) method called almost in every action/wait methods which internally calls 'root' method from 'PlaywrightToken' which is calling PlaywrightToken constructor which sets 'first' flag to false and due to this toLocator() method is returning locator instead of locator.first(). public Element click(String locator) { return new PlaywrightElement(this, ofRoot(locator)).click(); }

public static PlaywrightToken root(PlaywrightDriver driver, String token) { return new PlaywrightToken(driver, KarateTokenParser.toPlaywrightToken(token), null); }

public Locator toLocator() { Locator locator; if (getParent() == null) { locator = driver.rootLocator(getPlaywrightToken()); } else { locator = getParent().toLocator().locator(getPlaywrightToken()); } return (isFirst()) ? locator.first() : locator; } Possible Solution:- Can we call root method using PlayWright constructor with First flag set to true like public static PlaywrightToken root(PlaywrightDriver driver, String token) { return new PlaywrightToken(driver, KarateTokenParser.toPlaywrightToken(token), null, true); }

Also in some methods like text, value, attribute uses rootLocator which is not implemented for first locator , i guess.

Please correct me if I misunderstood

f-delahaye commented 9 months ago

You're right again! I don't know how I missed that, that's embarrassing...

I will take a closer look at the problem/your proposed solution.

azope25 commented 9 months ago

Also in some methods like text, value, attribute uses rootLocator which is not implemented for first locator , i guess.

@f-delahaye For rootLocator, Can we do return this.root.locator(locator).first();

f-delahaye commented 9 months ago

@azope25 yes, that's probably the easiest and quickest workaround. Do note however that scriptAll and locateAll probably will be broken with this change. I will have to refactor a few things for a complete fix.

Regarding passing true in the constructor, i'm a bit concerned about potential side effects. I think first() should be delayed as late as possible.

ptrthomas commented 6 months ago

@f-delahaye just posting here for future reference - something I ran into when I tried to run the build on docker:

command I ran: docker run -it --rm -v "$(pwd)":/src -w /src -v "$HOME/.m2":/root/.m2 maven:3-amazoncorretto-17 mvn clean install -P pre-release -pl -karate-robot

error:

[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running com.intuit.karate.playwright.driver.PlaywrightDriverTest
OpenJDK 64-Bit Server VM warning: Sharing is only supported for boot loader classes because bootstrap classpath has been appended
03:51:52.538 [main] DEBUG com.intuit.karate.shell.Command -- found / verified free local port: 4444
03:51:52.544 [main] DEBUG com.intuit.karate.playwright.driver.null_1709697112524 -- Playwright browsers will not be installed.
/tmp/playwright-java-14683935091175296828/node: /lib64/libm.so.6: version `GLIBC_2.27' not found (required by /tmp/playwright-java-14683935091175296828/node)
/tmp/playwright-java-14683935091175296828/node: /lib64/libc.so.6: version `GLIBC_2.28' not found (required by /tmp/playwright-java-14683935091175296828/node)
java.io.IOException: Stream closed
    at java.base/java.lang.ProcessBuilder$NullOutputStream.write(ProcessBuilder.java:445)
    at java.base/java.io.OutputStream.write(OutputStream.java:162)
    at java.base/java.io.BufferedOutputStream.flushBuffer(BufferedOutputStream.java:81)
    at java.base/java.io.BufferedOutputStream.flush(BufferedOutputStream.java:142)
    at com.microsoft.playwright.impl.WriterThread.run(PipeTransport.java:167)
java.io.IOException: Stream closed
    at java.base/java.lang.ProcessBuilder$NullOutputStream.write(ProcessBuilder.java:445)
    at java.base/java.io.OutputStream.write(OutputStream.java:162)
    at java.base/java.io.BufferedOutputStream.flushBuffer(BufferedOutputStream.java:81)
    at java.base/java.io.BufferedOutputStream.flush(BufferedOutputStream.java:142)
    at java.base/java.io.FilterOutputStream.close(FilterOutputStream.java:182)
    at com.microsoft.playwright.impl.PipeTransport.close(PipeTransport.java:93)
    at com.microsoft.playwright.impl.PipeTransport.poll(PipeTransport.java:71)
    at com.microsoft.playwright.impl.Connection.processOneMessage(Connection.java:200)
f-delahaye commented 6 months ago

Damn! Is that a blocking issue?

ptrthomas commented 6 months ago

@f-delahaye no not a blocking issue. must be some weirdness on linux + docker. so no worries for now

anildhiman88 commented 5 months ago

Hi @ptrthomas @f-delahaye , I have a very small query related to passing the context parameters in playwright options. i.e. playwrightOptions :{{igboreHTTPSErrors: true}}. But no luck, i believe it is not passed correctly to the context parameters in playwright driver.

ptrthomas commented 5 months ago

@f-delahaye this is also not a blocker, but I ran into another CI failure trying to get the build to work for Java 22 https://github.com/karatelabs/karate/issues/2542

[ERROR] Tests run: 4, Failures: 0, Errors: 4, Skipped: 0, Time elapsed: 3.155 s <<< FAILURE! -- in com.intuit.karate.playwright.driver.PlaywrightElementTest
[ERROR] com.intuit.karate.playwright.driver.PlaywrightElementTest.shiftTextEnter -- Time elapsed: 2.245 s <<< ERROR!
org.mockito.exceptions.base.MockitoException: 

Mockito cannot mock this class: class com.intuit.karate.playwright.driver.PlaywrightDriver.

If you're not sure why you're getting this error, please open an issue on GitHub.

am going to comment this out for now, FYI

f-delahaye commented 5 months ago

Hi @ptrthomas @f-delahaye , I have a very small query related to passing the context parameters in playwright options. i.e. playwrightOptions :{{igboreHTTPSErrors: true}}. But no luck, i believe it is not passed correctly to the context parameters in playwright driver.

Hi @anildhiman88 Sorry, I missed your comment. Indeed, ignoredHTTPSErrors is not supported. Playwright has many options, at different levels (browserType, browserContext...) and I'm not sure how to support them all in an easy and generic way. I'll try to find a solution.

ptrthomas commented 1 month ago

1.5.0 released