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

v5 - changelog, upgrade instructions, ready for tickets? #2943

Closed
dylang opened this issue Sep 7, 2018 · 6 comments
Closed

v5 - changelog, upgrade instructions, ready for tickets? #2943

dylang opened this issue Sep 7, 2018 · 6 comments

Comments

@dylang
Copy link
Contributor

dylang commented Sep 7, 2018

The problem

Hi! I'm new to webdriverio, and since I kept running in to issues that tickets said would be fixed in v5, I decided to give the beta a try. Exciting, I love the monorepo, the code seems a lot cleaner and easier to follow, and much more modern.

I'm not sure if you are ready for bugs, PR's, feature requests, etc for v5.

To close this ticket we just need some guidance with how you would like us involved in the beta testing of v5.

Environment

  • WebdriverIO version: v5
  • Node.js version: latest 10
  • Standalone mode or WDIO Testrunner: homemade jest test environment
  • if wdio testrunner, running synchronous or asynchronous tests: async
  • Additional wdio packages used (if applicable): not yet

Details

Since it took me a few hours to transition my code to 5.0.0-beta.1, I documented some of the changes I ran into so others could have a quicker upgrade.

Note to readers: I've only spent a couple days with v4 and a couple hours with v5 so these items might be incorrect, out-of-date, or subject to change.

  • There are no typescript types included and the typings from @types that target v4 are not compatible.
  • The sync: false config is ignored. Instead, your node_modules directory cannot have wdio-sync installed to enable async. This might be hard or impossible if you are using a monorepo and/or in the process of migrating. I used yarn why wdio-sync to find modules that were installing it and removed them.
  • Yarn was installing the RC1 of webdriver 5, which has a different API. Adding this to our package.json helped it install the right version.
"resolutions": {
    "webdriver": "5.0.0-beta.1"
}
  • No more browser.element or browser.elements, just browser.$/browser.$$.
  • No more browser.end. Use browser.deleteSession.
  • Element methods are no longer available from browser, such as browser.click(selector). (Cleaner/smaller API footprint, yay!)
  • waitForVisible changed to waitForDisplayed. Other element function names may have changed too.
  • In v4 async mode, if you resolved a $(selector) promise, it would break chaining and access to element methods. In v5 async mode, $(selector) must be resolved to access element methods like click(). Chaining async is not supported. This, of course, could lead to references becoming stale, so I'm guessing it will be fixed to support chaining. await browser.element(selector).click() becomes await (await browser.$(selector)).click().
  • As of beta.1, element timeouts exception stack is still not including the context, such as where in the test the timeout occurred. (This was one of the reasons I upgraded to v5.)
 FAIL  src/user/create-user.test.ts (27.553s)
  ● Create User › Creates a new User and Login

    element ("[data-test-name="new-user-button"]") still not existing after 20000ms

      at timer.catch.e (node_modules/webdriverio/src/commands/browser/waitUntil.js:60:19)

^ stack does not include where in the test file the request for the element occurred. This makes it more difficult to trace how far the test got when it failed.

Code To Reproduce Issue [ Good To Have ]

n/a

@dylang dylang changed the title v5 - changelog, upgrade instructions, what's not working yet v5 - changelog, upgrade instructions, ready for tickets? Sep 7, 2018
@christian-bromann
Copy link
Member

There are no typescript types included and the typings from @types that target v4 are not compatible.

Yes, with #2862 I am creating a script that should auto generate these type definitions. I am not an typescript expert so I will definitely need some help validating the result. You can also pick this task up, just let me know in the PR.

The sync: false config is ignored. Instead, your node_modules directory cannot have wdio-sync. This might be hard or impossible if you are using a monorepo and/or in the process of migrating.

True. I haven't actually thought about this. I guess it is impossible anyway to migrate and run v4 and v5 tests in the same repo anyway. What would you suggest. I definitely want to get rid of the configuration option.

Yarn was installing the RC1 of webdriver 5, which has a different API. Adding this to our package.json helped it install the right version.

Yeah, know issue. I deprecated rc1 of webdriver and released the actual version with @next tag. No idea why NPM is still preferring to install the deprecated version.

In v4 async mode, if you resolved a $(selector) promise, it would break chaining and access to element methods. In v5 async mode, $(selector) must be resolved to access element methods like click(). Chaining async is not supported. This, of course, could lead to references becoming stale, so I'm guessing it will be fixed to support chaining.

We got rid of support for promise chaining. It made the whole monad construct to complex to work with and it felt sometimes like pure magic. The issue with reference becomes stale doesn't really relate to this. It doesn't matter if you do:

await $("#elem").click()
# or
const elem = await $("#elem")
await elem.click()

It's the same thing. The first is not supported anymore.

waitForVisible changed to waitForDisplayed. Other function names may have changed too.

Yes, the new API can be discovered here: http://beta.webdriver.io/docs/api.html. We also haven't ported all commands but we will as we move forward with the project.

Methods for elements are no longer available from browser, such as browser.click(selector).

Nope. As mentioned above to access the element scope you need to actually fetch an element. No command that interacts with an element can be called from the browser object. This is also why we separated commands between browser and element.

No more browser.end. I'm using browser.closeWindow which seems to work, though I'm not sure if it's the right option.

The end command is now called deleteSession to make it more conform to the protocol. closeWindow just closes the window but not the WebDriver session.

As of beta.1, element timeouts don't in in the test the timeout occurred. (This was one of the reasons I upgraded to v5.

Not sure if I understand this one.

Thanks for trying out v5. Looking forward to more feedback!

@dylang
Copy link
Contributor Author

dylang commented Sep 7, 2018

Thanks @christian-bromann!

auto generate these type definitions`

Amazing! I'm new to Typescript as well, I'm using it to learn new libraries faster and write code with fewer bugs. It's been great, except supporting both sync and async api's in the same type definition kinda hampered some of the helpfulness because Typescript can't tell me what's a promise and what's value because everything can be both.

I definitely want to get rid of the [sync] configuration option.

I agree controlling something so pivotal to the entire system feels strange to be a simple boolean. What about publishing webdriverio-sync that has sync enabled, and webdriverio is only async?

  • This would make typing easier because the two packages could include only their respective types.
  • Same with docs. I would prefer a separate doc page (or a toggle) that only showed docs and examples in async since that's what I'm using.
  • Might make it easier for tickets tracking if they had different names rather than asking if wdio-sync was installed.

The end command is now called deleteSession to make it more conform to the protocol.

  • I've updated the issue with deleteSession to avoid confusing people.

This is also why we separated commands between browser and element.

I'm really happy about this change. The API had too many ways it could be used in v4 and I wasn't sure what the right approach was.

Not sure if I understand this one.

  • I tried to explain the timeout stack issue better above. Basically when $(selector) times out, I want to see the stack going all the way back to which line in my test had await $(selector).
at timer.catch.e (node_modules/webdriverio/src/commands/browser/waitUntil.js:60:19)

This stack tells me about webdriverio's code, not my code.

It's the same thing. The first is not supported anymore.

const elem = await $("#elem")
await elem.click()

This seems fine, but over time the code evolves to something like this:

// await $("#elem").click() // v4 api
const elem = await $("#elem")
await elem.click()
// fill out form...
// submit form...
// await $("#elem").click() // v4 api
await elem.click() // Oh no, this could be a stale reference to "#elem"!

or do we need to do something like this to avoid stale references:

// await $("#elem").click() // v4 api
const elem = await $("#elem")
await elem.click()
// fill out form....
// submit form...
// await $("#elem").click() // v4 api
const elemAgain = await $("#elem")
await elemAgain.click() // we're safe

@christian-bromann
Copy link
Member

What about publishing webdriverio-sync

I don't think this is a good idea. In general I would like to advocate that it is always better to use sync because it is (especially now with v5) literally just removing the await in front of the command which makes the code so much easier to read.

This stack tells me about webdriverio's code, not my code.

That would be indeed an issue. I guess we can tweak the stack trace that it reports correctly.

or do we need to do something like this to avoid stale references:

For this we actually implemented a retry mechanism. So if a stale element errors occurs it refetches the selector and updates the element automatically. I think someone started the work on that, I can't find the issue though.

@dylang
Copy link
Contributor Author

dylang commented Sep 7, 2018

I don't think this is a good idea. In general I would like to advocate that it is always better to use sync because it is (especially now with v5) literally just removing the await in front of the command which makes the code so much easier to read.

Interesting. I thought fibers were a hack in node. I agree it's easier to read. I think the v4 remote api was async-only. A team here using Cucumber required us to only use async so we can share our page objects.

What about webdriverio-async?

For this we actually implemented a retry mechanism. So if a stale element errors occurs it refetches the selector and updates the element automatically.

Nice! Would that include retrying parent selectors? Example:

const parent = await $('parent');
const child = await parent.$(child);
.... // React re-renders parent and child
await child.click(); // would need to retry parent to find child.

@christian-bromann
Copy link
Member

Nice! Would that include retrying parent selectors?

Yes.

@dylang anything else to answer? Happy to do this also on the Gitter support channel.

@christian-bromann
Copy link
Member

If someone has questions to the new v5 version feel free to join the dev channel Gitter. Thanks for reaching out @dylang

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

No branches or pull requests

2 participants