Closed Bug 1204504 Opened 5 years ago Closed 4 years ago

Illegal exception is thrown on an invalid locator

Categories

(Testing :: Marionette, defect)

43 Branch
defect
Not set
normal

Tracking

(firefox46 fixed)

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: barancev, Assigned: ato)

References

(Blocks 1 open bug)

Details

(Keywords: pi-marionette-server)

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/45.0.2454.85 Safari/537.36

Steps to reproduce:

Search by an invalid locator


Actual results:

General webdriver error occurs:

INFO: Executing: findElement [e6e35c7c-0cff-4d9e-96f0-94a4a00cf6f4, findElement {using=css selector, value=#}]
1442239082133	Marionette	DEBUG	conn0 -> {"name":"findElement","parameters":{"using":"css selector","value":"#"},"sessionId":"e6e35c7c-0cff-4d9e-96f0-94a4a00cf6f4"}
1442239082339	Marionette	DEBUG	conn0 client <- {"error":"webdriver error","message":"An invalid or illegal string was specified","stacktrace":null}
Marionette threw an error: SyntaxError: An invalid or illegal string was specified
EM_findElement@chrome://marionette/content/elements.js:620:19
EM_find@chrome://marionette/content/elements.js:485:23
findElementContent/<@chrome://marionette/content/listener.js:1403:1
findElementContent@chrome://marionette/content/listener.js:1402:1
dispatch/</req<@chrome://marionette/content/listener.js:165:22
TaskImpl_run@resource://gre/modules/Task.jsm:314:40
TaskImpl@resource://gre/modules/Task.jsm:275:3
createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:249:14
Task_spawn@resource://gre/modules/Task.jsm:164:12
dispatch/<@chrome://marionette/content/listener.js:160:15



Expected results:

Invalid selector error should be returned
http://www.w3.org/TR/webdriver/#handling-errors
http://www.w3.org/TR/webdriver/#element-location-strategies
Assignee: nobody → ato
Blocks: webdriver
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Bug 1204504: Wrap invalid CSS query error in invalid selector error

Thanks to Alexei Barantsev for reporting.

r=dburns
Attachment #8662332 - Flags: review?(dburns)
Comment on attachment 8662332 [details]
MozReview Request: Bug 1204504 - Error on invalid selector; r=automatedtester

https://reviewboard.mozilla.org/r/19579/#review17541

::: testing/marionette/client/marionette/tests/unit/test_findelement.py:84
(Diff revision 1)
> +    def test_invalid_css_selector(self):

Please make the name slightly more descriptive.

::: testing/marionette/client/marionette/tests/unit/test_findelement.py:85
(Diff revision 1)
> +        with self.assertRaises(InvalidSelectorException):

This can be done on one line

::: testing/marionette/client/marionette/tests/unit/test_findelement.py:163
(Diff revision 1)
> -        test_html = self.marionette.absolute_url("test.html")
> +        with self.assertRaises(InvalidSelectorException):

This can be done on one line

Update the minor issues and good to go.
Attachment #8662332 - Flags: review?(dburns) → review+
Comment on attachment 8662332 [details]
MozReview Request: Bug 1204504 - Error on invalid selector; r=automatedtester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/19579/diff/1-2/
Attachment #8662332 - Attachment description: MozReview Request: Bug 1204504: Wrap invalid CSS query error in invalid selector error → MozReview Request: Bug 1204504 - Error on invalid selector; r=automatedtester
https://reviewboard.mozilla.org/r/19579/#review17541

> This can be done on one line

I find the one line version, where you pass the function reference and its arguments, and unittest will invoke it for you more unreadable because you need to understand what unittest does with it.

Doing it like this is clearer and more understandable, in my opinion, because it removes any doubt what is actually executed.

> This can be done on one line

I don’t particularly like the reflection style API that assertRaises provides to make this a single line.

IMO it adds an additional layer of abstraction that is unnecessary.
https://hg.mozilla.org/mozilla-central/rev/d7025fe68e90
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.