1092812 - Test method assertHighlight is not really asserting if there are nodes highlighted

Status: RESOLVED DUPLICATE of bug 1088706

(Firefox OS Graveyard :: Gaia::Contacts, defect)

Description

[Pedro Cambra]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:36.0) Gecko/20100101 Firefox/36.0
Build ID: 20141101030205

Steps to reproduce:

Run assertHighlight with any string that should be not found, such as:

assertHighlight('LALALA');


Actual results:

Test passes


Expected results:

Test should fail

Comment 1

[Pedro Cambra]

Working on this

Comment 2

[Pedro Cambra]

Attachment: Adding a check on how much nodes were actually highlighted

This is going to break all tests that were relying blindly on assertHighlight.

Opening a new bug for this

Comment 3

[Pedro Cambra]

https://bugzilla.mozilla.org/show_bug.cgi?id=1092816

Comment 4

[Adrián de la Rosa]

I have been researching a bit about this bug (didn't know you were already working on it) and I can see that your patch makes obvious that the tests are checking anything. What worries me is that searchList.getElementsByClassName(highlightClass) never returns any node, and I don't know why, but it can have something to do with highlighting using clones of the nodes and not the real nodes. 

I think tests should be passing once assertHighlight is fixed, because the problem relies on this function.

Comment 5

[Pedro Cambra]

I've opened an additional bug for fixing the tests (referenced above: 1092816).

As you say, searchList.getElementsByClassName(highlightClass) never returns any node. That's because, in the tests (and just there), the highlight span is never added to the text. I was working on that in 1092816

Comment 6

[Adrián de la Rosa]

I saw you can't assign yourself to bugs so I assigned you. I also set 1092816 as a blocker for this bug. Please feel free to ask if you want to change something else or revert some of these changes I made.

Thanks a lot for your work!

Comment 7

[Adrián de la Rosa]

I've been researching a bit more and I am doing a little refactor of the search tests in this test file because I've seen several worrying issues with them. I will add the refactor to my pull request in bug 1088706.

Comment 8

[Jose Manuel Cantera]

we are fixing this on bug 1088706

Comment 9

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8515624 [details] [review]
Adding a check on how much nodes were actually highlighted

Removing the review flag