Skip to content

Conversation

jimevans
Copy link
Contributor

The structure of responses for successfully executed commands is not specified in the current draft. This pull request remedies that situation.

@illicitonion
Copy link

What's the purpose of the guaranteed-singleton wrapping map?

@andreastt
Copy link
Member

We don't need to say anything special about success responses because each command (is supposed to) define the type and data structure of their own return value. That means we can take the return value without knowing anything about it and assign it directly to the response's body.

Examples:

Commands with no return value also shouldn't have a JSON object with a value property, as the only way we can distinguish between “ok response” and “value response” is by checking if it the response has a body. E.g. something returning null isn't quite the same as OK.

@jimevans
Copy link
Contributor Author

Aside from not guaranteeing the object at the root being entirely inconsistent with existing implementations, having a consistent structure makes the response body more easily consumable by (some) local end implementations. This consistent structure includes being able to depend on the success response being an object at its root, rather than one of a scalar value of type string, number, or boolean; a null; an array; or an object.

@jgraham
Copy link
Member

jgraham commented Apr 20, 2015

I think there is a bug in the current specification; the response should be the serialization of an empty object when there is no data. I don't see what {"value":null} would add add, and it is inconsistent with my understanding of how more complex responses like a cookie value should work (with a single object like {name:something, path:whatever, [...]} rather than a pointlessly nested object like {value:{name: something, path: whatever, [...]}})

@jimevans
Copy link
Contributor Author

@jgraham As I said, the structure makes it easier for some local end language bindings to consume, as well as being externally consistent with both existing implementations and internally consistent with the spec's own commands (c.f. findElement). I don't find the wrapping pointless for those reasons. However, if neither ease of local end consumption nor consistency with existing implementations are goals of the spec any longer, I'll withdraw the pull request.

@AutomatedTester Would a better approach be to submit pull requests or bugs to flesh out individual commands that do not, at present, properly specify their return content (e.g., getTitle)?

@AutomatedTester
Copy link
Contributor

consistency with existing implementations

This has never been one of the goals of the specification. The goal is to document what we want WebDriver to be and to fix implementations along the way. We are already have diverged from the implementation with the transport layer, having a lighter response (removing one layer of the dictionary) does feel like the next step.

Would a better approach be to submit pull requests or bugs to flesh out individual commands that do not, at present, properly specify their return content (e.g.,getTitle)?

getCurrentUrl shows an example of using value as a key if we don't a complex object to return. Does this satisfy your requirement for calls that don't have a specific key?

@jimevans
Copy link
Contributor Author

No, in point of fact, it doesn't satisfy my requirement. The return value getCurrentUrl is a fine example for that particular command. If all other commands were similarly documented, or if there were a global statement to the effect of, "unless otherwise noted for specific commands, success responses will look thus," my requirements would be met. At present, however, all commands do not explicitly specify the structure of their return data, and a global caveat such as mentioned above would render the spec less readable in my opinion. So my question still stands: How would you like to handle insufficiently specified response values for successful commands in the spec, as bugs in bugzilla, or as pull requests to the spec?

@jgraham
Copy link
Member

jgraham commented Apr 28, 2015

At this point I think it's a known fact that every command after getCurrentUrl, excluding the cookie functionality, needs to be rewritten to match the quality bar for the spec.

@AutomatedTester
Copy link
Contributor

How would you like to handle insufficiently specified response values for successful commands in the spec, as bugs in bugzilla, or as pull requests to the spec?

As was discussed in London we need to rewrite pretty much every command to be a lot more precise. We raised Bug 26275 related to this so we don't need any more bugs. If you want to do pull requests for each of the commands to make them more precise please do feel free to do so. I am not inclined to add a global version at the moment when we are working through the rest of the specification to bring it up to the level it should be.

@jimevans jimevans closed this May 1, 2015
@jimevans jimevans deleted the success branch February 23, 2017 17:49
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.

5 participants