Skip to content
This repository has been archived by the owner on Oct 25, 2023. It is now read-only.

fix: Patch downstream responses for unknown upstream protocol #353

Merged
merged 3 commits into from
Sep 3, 2019

Conversation

mykola-mokhnach
Copy link
Contributor

the idea is to add status and sessionId fields to getStatus responses if the call was made before session creation request has been sent
Addresses appium/appium#13161

Copy link
Member

@KazuCocoa KazuCocoa left a comment

Choose a reason for hiding this comment

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

Do we have test cases for getStatus in this repo?
If we had, adding an assertion about it, like if it has 'status', is pretty good

otherwise lgtm

@@ -199,6 +212,11 @@ class ProtocolConverter {
*/
async convertAndProxy (commandName, url, method, body) {
if (!this.downstreamProtocol) {
if (commandName === 'getStatus') {
// Patch getStatus call for GENERIC protocol
Copy link
Member

Choose a reason for hiding this comment

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

👀
ah...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are existing tests, I would say they do cover such changes

@mykola-mokhnach mykola-mokhnach merged commit 9af6258 into appium:master Sep 3, 2019
@mykola-mokhnach mykola-mokhnach deleted the status branch September 3, 2019 05:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants