Skip to content

Update launcher.ts to handle defaultViewport#10883

Merged
christian-bromann merged 10 commits intowebdriverio:mainfrom
mighty98:#10866
Aug 8, 2023
Merged

Update launcher.ts to handle defaultViewport#10883
christian-bromann merged 10 commits intowebdriverio:mainfrom
mighty98:#10866

Conversation

@mighty98
Copy link
Contributor

@mighty98 mighty98 commented Aug 7, 2023

Proposed changes

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

@mighty98
Copy link
Contributor Author

mighty98 commented Aug 7, 2023

@christian-bromann Im not sure if my changes are causing the test failure. If you could guide me will fix it

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.

Do you think we can add a unit tests for this? Ideally somewhere in packages/devtools/tests/launcher.test.ts

mighty98 and others added 3 commits August 7, 2023 22:56
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.

Awesome, LGTM 👍

@christian-bromann christian-bromann added the PR: Polish 💅 PRs that contain improvements on existing features label Aug 7, 2023
@christian-bromann
Copy link
Member

You can update the snapshots by calling npx vitest -u

@mighty98
Copy link
Contributor Author

mighty98 commented Aug 7, 2023

@christian-bromann I see test failures
image
I see the issues are caused due to the position of these variables.
I can change my code to put these in the same position as before. Should I?

@mighty98
Copy link
Contributor Author

mighty98 commented Aug 8, 2023

@christian-bromann Not sure if the failed tests are due to my changes.. I could not find any relations

@christian-bromann
Copy link
Member

The build is currently broken which needs to get fixed. I will go ahead and merge this one. Thanks for all your work!

@christian-bromann christian-bromann merged commit aeb6e64 into webdriverio:main Aug 8, 2023
@mighty98
Copy link
Contributor Author

mighty98 commented Aug 8, 2023

Thank you @christian-bromann

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

Labels

PR: Polish 💅 PRs that contain improvements on existing features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants