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

feat: add all the types #635

Merged
merged 19 commits into from
Oct 13, 2023
Merged

feat: add all the types #635

merged 19 commits into from
Oct 13, 2023

Conversation

boneskull
Copy link
Contributor

@boneskull boneskull commented Jun 30, 2023

  • uses TS instead of babel
  • adds the usual tooling (no renovate)

this depends on appium/appium#18817 and appium/appium-android-driver#819.

  • all mixins were changed to be object literals.
  • because of the high levels of confusion around the capability objects being built in createSession, I renamed some of the methods it calls and attempted to otherwise make things more clear. createSession returns an object which is a superset of the user-defined capabilities, since it adds a bunch of device info. see lib/types.ts for the type definitions of these objects; createSession responds with a capabilities object of type Uiautomator2SessionCaps
  • renamed fillDeviceDetails to getDeviceDetails and hopefully made it faster by running the async operations in parallel.
  • initUiautomator2Server returns the Uiautomator2Server instance (which we consume) so we know the value is defined. otherwise it still mutates the instance.

classes can use the CreateResult type argument when extending BaseDriver and implementing ExternalDriver to resolve with a custom value; if unset, the returned capabilities will contain only props as defined in the driver's constraints (plus any "base" constraints).

@boneskull
Copy link
Contributor Author

boneskull commented Jun 30, 2023

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@boneskull boneskull marked this pull request as ready for review July 5, 2023 18:34
@mykola-mokhnach
Copy link
Contributor

I cannot review the PR, - it's too big 🤷
In such case I usually merge yolo style and then monitor for possible issues to quickly fix them

boneskull and others added 9 commits July 5, 2023 14:18
both the functional test and unit test workflows had a job called "test" which was confusing act
also tweak some tests
- also upgrades chromedriver
- fixes some broken types
- re-add `source-map-support`
@jlipps
Copy link
Member

jlipps commented Sep 20, 2023

OK @mykola-mokhnach I merged in master and made all the required fixes. Let's see what tests say.

@jlipps
Copy link
Member

jlipps commented Sep 21, 2023

tests seem reasonably ok. propose that we merge and cut a beta release and get people to test it out

@mykola-mokhnach
Copy link
Contributor

@jlipps Yes, lets do it. Do you also have a similar PR for Espresso?

@jlipps
Copy link
Member

jlipps commented Sep 22, 2023

@mykola-mokhnach no, nothing for Espresso yet. We'll need to work on that one from scratch. Or does it rely on the uia2 driver at all? Also, I am on PTO monday so I don't think it's a good idea for me to merge and publish. I'm also not sure if there is some kind of auto release script on this repo. Do you want to handle the mechanics of the beta release?

@mykola-mokhnach
Copy link
Contributor

@mykola-mokhnach no, nothing for Espresso yet. We'll need to work on that one from scratch. Or does it rely on the uia2 driver at all? Also, I am on PTO monday so I don't think it's a good idea for me to merge and publish. I'm also not sure if there is some kind of auto release script on this repo. Do you want to handle the mechanics of the beta release?

This repo has autorelease based on conventional commits (e.g. PR title and description).
Espresso driver does not have anything to do with UIA2 one, although I think it for sure makes sense to also update it if we want to release v6 of the android driver. Otherwise it will stop getting shared updates.

It makes sense to release the driver if you are around, so please do it before you know you will be available for a while.

@mykola-mokhnach mykola-mokhnach changed the title chore: add all the types refactor!: add all the types Oct 13, 2023
@mykola-mokhnach mykola-mokhnach changed the title refactor!: add all the types refactor: add all the types Oct 13, 2023
@mykola-mokhnach mykola-mokhnach changed the title refactor: add all the types feat: add all the types Oct 13, 2023
@mykola-mokhnach mykola-mokhnach merged commit 623f49b into master Oct 13, 2023
13 of 14 checks passed
@mykola-mokhnach mykola-mokhnach deleted the boneskull/types branch October 13, 2023 21:01
github-actions bot pushed a commit that referenced this pull request Oct 13, 2023
## [2.30.0](v2.29.11...v2.30.0) (2023-10-13)

### Features

* add all the types ([#635](#635)) ([623f49b](623f49b))
@github-actions
Copy link

🎉 This PR is included in version 2.30.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@pavlo-dot-dev
Copy link

I have error "Error: Failed to create session. A new session could not be created. Details: 'app' can't be blank" on this version, when i try make session.

@pavlo-dot-dev
Copy link

#641 is actually problem in this version

@mykola-mokhnach
Copy link
Contributor

@pavlo-dot-dev Please report your issue to the appropriate tracker and add the environment info plus server logs

@steven-george-neurovalens

Disclaimer - First time poster - @boneskull I tried to move from uiautomator2@2.29.9 to the latest (uiautomator2@2.43.4) this version appears to be where driver.executeScript("mobile: deviceInfo", "networks") starts throwing errors for me. Was this collateral or an unreported issue? Sorry if it's been previous addressed I did try to look and search and didn't find any mention.

@KazuCocoa
Copy link
Member

mobile: deviceInfo itself expects to have no arguments. What about removing "networks"? The response could have network info etc -> https://github.com/appium/appium-uiautomator2-driver?tab=readme-ov-file#mobile-deviceinfo
(potentially old implementation accepted arguments but ignored internally)

@steven-george-neurovalens

@KazuCocoa thanks for steer - shouldn't really have needed it but here we are - taking out "networks" got me back what I used to use / needs. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants