Skip to content

Support for log_event and get_events command#469

Merged
ki4070ma merged 15 commits intoappium:masterfrom
ki4070ma:fix-events
Nov 18, 2019
Merged

Support for log_event and get_events command#469
ki4070ma merged 15 commits intoappium:masterfrom
ki4070ma:fix-events

Conversation

@ki4070ma
Copy link
Copy Markdown
Collaborator

@ki4070ma ki4070ma commented Nov 9, 2019

Comment thread appium/webdriver/webdriver.py Outdated
Comment thread appium/webdriver/webdriver.py Outdated
@ki4070ma ki4070ma changed the title Use appium/events as endpoint to get events [WIP] Use appium/events as endpoint to get events Nov 9, 2019
@ki4070ma ki4070ma changed the title [WIP] Use appium/events as endpoint to get events Support for log_event and event command Nov 10, 2019
Comment thread appium/webdriver/extensions/log_event.py Outdated
Comment thread appium/webdriver/extensions/log_event.py Outdated
Comment thread appium/webdriver/extensions/log_event.py Outdated
Comment thread appium/webdriver/extensions/log_event.py Outdated
@mykola-mokhnach
Copy link
Copy Markdown
Contributor

cc @dpgraham

Can you please take a look at this change? I remember you were working on #429

@mykola-mokhnach
Copy link
Copy Markdown
Contributor

I am ok with the PR, although I would like @dpgraham to check it before merging, since it might be a breaking one

@ki4070ma ki4070ma requested a review from dpgraham November 10, 2019 12:17
class LogEvent(webdriver.Remote):

@property
def events(self):
Copy link
Copy Markdown
Member

@KazuCocoa KazuCocoa Nov 10, 2019

Choose a reason for hiding this comment

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

https://github.com/appium/appium-base-driver/blob/master/lib/protocol/routes.js#L589-L594
/events route accepts type argument as option.
It hasn't been implemented yet, but this events should not be property.

You can rename here as get_events(self, type = None) to keep another events as session_capability['events']. It is for backword compatibility as #469 (comment)

Copy link
Copy Markdown
Collaborator Author

@ki4070ma ki4070ma Nov 10, 2019

Choose a reason for hiding this comment

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

@KazuCocoa
Thanks, I misunderstood, I will keep original events and create get_events as another one.

@ki4070ma ki4070ma changed the title Support for log_event and event command Support for log_event and get_events command Nov 10, 2019
Comment thread appium/webdriver/webdriver.py
It isn't implemented yet for now
startTime: (int) Received time
endTime: (init) Response time
"""
return self.execute(Command.GET_EVENTS)['value']
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As a client, below two cases are available

self.execute(Command.LOG_EVENT, {})
self.execute(Command.LOG_EVENT, {'type': nil}) (<= Appium server will handle this 'type' to filter events)

Copy link
Copy Markdown
Collaborator Author

@ki4070ma ki4070ma Nov 10, 2019

Choose a reason for hiding this comment

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

Yes, I've checked and added, but removed by below commit since it isn't implemented yet.
In my opinion, it's enough to add type when it's ready.

0a84069

However, I'll restore type along to your comments. Any other comments are welcome.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It could depend on server-side if the parameter works.
We can handle the behaviour only server-side modification (and documentation on appium.io), if clients follow the route definition like other commands.

The value isn't passed to the server for now.
@KazuCocoa
Copy link
Copy Markdown
Member

Added type.
Please check the description in appium/appium-base-driver#368 about the available format

@ki4070ma ki4070ma requested a review from KazuCocoa November 16, 2019 15:09
@ki4070ma ki4070ma merged commit b797254 into appium:master Nov 18, 2019
@ki4070ma ki4070ma deleted the fix-events branch November 18, 2019 14:51
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

Successfully merging this pull request may close these issues.

3 participants