Skip to content

Support for running WebdriverIO TypeScript tests on Node 20. Fixes #10901#11178

Closed
jan-molak wants to merge 5 commits intowebdriverio:mainfrom
jan-molak:fix/issue-10901
Closed

Support for running WebdriverIO TypeScript tests on Node 20. Fixes #10901#11178
jan-molak wants to merge 5 commits intowebdriverio:mainfrom
jan-molak:fix/issue-10901

Conversation

@jan-molak
Copy link
Contributor

@jan-molak jan-molak commented Sep 16, 2023 •
edited
Loading

Proposed changes

A more targeted variation of #11173, aimed at resolving #10901.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Checklist

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • I have added proper type definitions for new commands (if appropriate)

Further comments

Reviewers: @webdriverio/project-committers

@jan-molak jan-molak marked this pull request as draft September 16, 2023 15:19
@jan-molak jan-molak marked this pull request as ready for review September 16, 2023 16:15
@jan-molak
Copy link
Contributor Author

jan-molak commented Sep 16, 2023

@christian-bromann e2e-mac failure seems to have something to do with node module caching, perhaps?

christian-bromann
christian-bromann approved these changes Sep 18, 2023
View reviewed changes
Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@erwinheitzman
Copy link
Member

erwinheitzman commented Sep 18, 2023

Why do we install Serenity just to test some functionality? I suggest that we replace it with a check of our own

@jan-molak
Copy link
Contributor Author

jan-molak commented Sep 18, 2023 •
edited
Loading

Why do we install Serenity just to test some functionality? I suggest that we replace it with a check of our own

Hey @erwinheitzman - This is to avoid changes to WDIO / TS-Node integration breaking WDIO / Serenity/JS integration. Adding a single dev dependency to be used in an integration test seemed like a reasonable way to enable a fast feedback loop for the WDIO project. Plus, it follows the same pattern WDIO uses to test integration with the Testing Library and other frontend frameworks like Lit, Preact, React, Svelte and Vue.

I hope this explanation helps, but let me know if you have any other questions.

Also see #10816 for reference.

@christian-bromann
Copy link
Member

christian-bromann commented Sep 18, 2023

@jan-molak it seems like the added e2e tests are failing, mind taking a look?

@jan-molak
Copy link
Contributor Author

jan-molak commented Sep 18, 2023

@christian-bromann - it looks like the failure of e2e-windows is unrelated to the change I proposed:

[2-0] RUNNING in firefox - file:///D:/a/webdriverio/webdriverio/e2e/wdio/headless/secondTest.e2e.ts
[0-0] RUNNING in chrome - file:///D:/a/webdriverio/webdriverio/e2e/wdio/headless/secondTest.e2e.ts
[1-0] RUNNING in chrome - file:///D:/a/webdriverio/webdriverio/e2e/wdio/headless/secondTest.e2e.ts
[3-0] RUNNING in firefox - file:///D:/a/webdriverio/webdriverio/e2e/wdio/headless/secondTest.e2e.ts
[4-0] RUNNING in edge - file:///D:/a/webdriverio/webdriverio/e2e/wdio/headless/secondTest.e2e.ts
[4-0] FAILED in edge - file:///D:/a/webdriverio/webdriverio/e2e/wdio/headless/secondTest.e2e.ts
[1-0] PASSED in chrome - file:///D:/a/webdriverio/webdriverio/e2e/wdio/headless/secondTest.e2e.ts
[0-0] PASSED in chrome - file:///D:/a/webdriverio/webdriverio/e2e/wdio/headless/secondTest.e2e.ts
[3-0] PASSED in firefox - file:///D:/a/webdriverio/webdriverio/e2e/wdio/headless/secondTest.e2e.ts
[2-0] PASSED in firefox - file:///D:/a/webdriverio/webdriverio/e2e/wdio/headless/secondTest.e2e.ts

and the e2e-mac build fails because it can't find the new dependency, even though the windows one can... It seems rather odd, but maybe I'm misunderstanding how deps are installed in the WDIO build on CI 🤔

Let me take another stab at this on a fresh branch

@jan-molak jan-molak marked this pull request as draft September 18, 2023 20:33
@jan-molak
Copy link
Contributor Author

jan-molak commented Sep 19, 2023 •
edited
Loading

Closing in favour of #11202

@jan-molak jan-molak closed this Sep 19, 2023
@jan-molak jan-molak mentioned this pull request Sep 19, 2023
Merged
13 tasks
@jan-molak jan-molak deleted the fix/issue-10901 branch September 19, 2023 01:05
@erwinheitzman
Copy link
Member

erwinheitzman commented Sep 19, 2023

Why do we install Serenity just to test some functionality? I suggest that we replace it with a check of our own

Hey @erwinheitzman - This is to avoid changes to WDIO / TS-Node integration breaking WDIO / Serenity/JS integration. Adding a single dev dependency to be used in an integration test seemed like a reasonable way to enable a fast feedback loop for the WDIO project. Plus, it follows the same pattern WDIO uses to test integration with the Testing Library and other frontend frameworks like Lit, Preact, React, Svelte and Vue.

I hope this explanation helps, but let me know if you have any other questions.

Also see #10816 for reference.

Thanks for the reply, makes sense 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@christian-bromann christian-bromann christian-bromann approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jan-molak @erwinheitzman @christian-bromann