1231979 - test_clock_set_alarm.py : AssertionError: u'\u206810\u2069 minutes' != '10 minutes'

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::UI Tests, defect)

Description

[Max Ivanov[:MaxIvanov]]

Description: Test case fails with "AssertionError"

Steps:
1. Update a flame device to 20151211030233
2. Run this test case http://lxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/tests/functional/clock/test_clock_set_alarm.py

Actual result:
"AssertionError"

Expected result:
Test completes successfully.

Repro Rate: Reproduced via Jenkins adhoc (0/5 passing)
http://jenkins1.qa.scl3.mozilla.com/job/flame-kk.ui.adhoc.bitbar/371/HTML_Report/

Traceback (most recent call last):
File "/var/lib/jenkins/jobs/flame-kk.ui.adhoc.bitbar/workspace/.env/lib/python2.7/site-packages/marionette_client-2.0.0-py2.7.egg/marionette/marionette_test.py", line 344, in run
testMethod()
File "/var/lib/jenkins/jobs/flame-kk.ui.adhoc.bitbar/workspace/tests/python/gaia-ui-tests/gaiatest/tests/functional/clock/test_clock_set_alarm.py", line 48, in test_clock_set_alarm
self.assertEquals(new_alarm.alarm_snooze, '10 minutes')
AssertionError: u'\u206810\u2069 minutes' != '10 minutes'

Comment 1

[Max Ivanov[:MaxIvanov]]

Attachment: index.png

Comment 2

[GitHub Autolander Bot]

Attachment: [gaia] mwargers:1231979 > mozilla-b2g:master

Comment 3

[No-Jun Park [:njpark]]

Comment on attachment 8704240 [details] [review]
[gaia] mwargers:1231979 > mozilla-b2g:master

lgtm

Comment 4

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8704240 [details] [review]
[gaia] mwargers:1231979 > mozilla-b2g:master

Crap, this type of issue is becoming more recurring.

We need a smarter way to solve it. Otherwise the readability of the tests will be hurt. For example, we need to keep the error message free from mindtricks and other vile traps, unlike:
> AssertionError: u'\u206815\u2069 minutes' doesn't equal u'\u2068115\u2069 minutes'

Unicode characters #2068 and #2069 are invisible characters that help (if I understood correctly) making numbers still in LTR for RTL languages. 

HtmlElement.text is supposed to give us the visible text, though. Then, I would suggest to make patch in marionette client to show what's really visible.

Comment 5

[Martijn Wargers (dead)]

(In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #4)
> Comment on attachment 8704240 [details] [review]
> HtmlElement.text is supposed to give us the visible text, though. Then, I
> would suggest to make patch in marionette client to show what's really
> visible.

That's what it's using internally, see: http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/apps/clock/regions/new_alarm.py#47

Comment 6

[Johan Lorenzo [:jlorenzo]]

Yeah, that's why I suggested to patch marionette-client. After a discussion with AutomatedTester, that's something that needs to be done in Selenium itself[1]. But it needs to be specified in W3C first. Bug 883555 tracks that for us.

Then, the patch will likely take longer to be made. In this case, I'd recommend to use "data-l10n-args" and make sure we have "15" in it. This would also make the test more locale-agnostic.

What do you think? 

[1] https://github.com/SeleniumHQ/selenium/blob/ab1e647d0fc8fc39e6b00ae94321ab228b6728f2/javascript/selenium-atoms/text.js#L149
[2] https://www.w3.org/Bugs/Public/show_bug.cgi?id=25058

Comment 7

[Martijn Wargers (dead)]

Patching marionette-client is going to be very difficult, from what I understood. It involves changing atoms.js, which is a huge minified js file and is very difficult to update, see bug 1203966, comment 2 and further. But perhaps I'm misunderstanding.

I guess I'll go for the data-l10n attribute then

Comment 8

[Andreas Tolfsen ❲:ato❳]

The long-term intention of the WebDriver WG is to rely on a web standard to tell us the text-serialisation of a node.

roc’s been working on a specification for innerText, which is already supported by our friends working on other browsers.  I expect current implementations of innerText to have significant interoperability issues right now, however this spec could help move vendors in the right direction.

mwargers/jlorenzo: I haven’t studied the innerText prose in depth at all.  Can you tell if it addresses the issue that the Selenium atom has with regards to bi-directional whitespace characters?

innerText is already available in Nightly and it would be useful if someone could do the legwork and compare it against the behaviour of the current atom implementation.

Comment 9

[David Burns :automatedtester]

Andreas has the same comment as me so removing NI

Comment 10

[Martijn Wargers (dead)]

Should be fixed now with the merge of the first pull request in bug 1219840.