[mozmill] Assertion failure in testSuggestHistoryBookmarks.js

RESOLVED FIXED

Status

RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: ashughes, Assigned: gmealer)

Tracking

({regression})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mozmill-test-failure], URL)

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

8 years ago
MODULE: /firefox/testAwesomeBar/testSuggestHistoryBookmarks.js   
TEST:   testStarInAutocomplete (107)    
FAIL:   Controller.assertJS("subject.isItemBookmarked == true")
BRANCH: default, 1.9.1
PLATFORMS: All
(Reporter)

Updated

8 years ago
Whiteboard: [mozmill-test-failure]
(Reporter)

Comment 1

8 years ago
This failure is default only.  The failure on 1.9.1 is completely different.

On default, watching this appears that the awesombebar autocomplete does not drop down at all.  There are also no entries for Litmus in the history.  This will need further investigation.

I'll file a bug for the 1.9.1 issue.
(Reporter)

Updated

8 years ago
Assignee: nobody → anthony.s.hughes
Status: NEW → ASSIGNED
(In reply to comment #1)
> I'll file a bug for the 1.9.1 issue.

Please assign yourself to this newly created bug for other platforms. Geo will work on the failure on trunk which is clearly an API change.
Assignee: anthony.s.hughes → gmealer
(Reporter)

Comment 3

8 years ago
See bug 573618 for the 1.9.1 issue.
I ran through this one manually, don't see the bookmarked item appearing in the awesomebar.  I think the test is failing because there's an actual Minefield bug.  Am communing with Henrik to determine next steps.
I'm sure this was caused by the implementation of the "Switch to Tab" feature on bug 564573. We should find out if that behavior is wanted and update our test. We will have to close the tab with the bookmarked page before we can enter the term into the location bar.
Blocks: 564573
Keywords: regression
(In reply to comment #5)
> I'm sure this was caused by the implementation of the "Switch to Tab" feature
> on bug 564573. We should find out if that behavior is wanted and update our
> test. We will have to close the tab with the bookmarked page before we can
> enter the term into the location bar.

Alex, can you confirm this is the wanted behavior? We don't show any annotation icons on the right hand side for URLs which are open in tabs.
Yes, the switch-to-tab entry should have the same decorations as any other entry (e.g. star, tags, etc). The only difference is that it doesn't show the URL, and shows "Switch to Tab" instead.
Depends on: 575609
I am still seeing this test failure, but I know why it's failing.

The reason it is failing is because the bookmarked star icon is not visible in the autocomplete as a result of us already sitting on the Litmus page, alongside the changes in Switch to Tab. When we type 'Litmus' in the address bar, it simply lets us go to the Switch To Tab, i.e., no other decorations are present. This test will pass if we go Home, or change to about:blank in the second test function: testStarInAutocomplete.

I'm not sure which Litmus test this corresponds with, but it would need to reflect the changes with Switch to Tab, and then we can fix this test with the fix above.
(In reply to comment #7)
> Yes, the switch-to-tab entry should have the same decorations as any other
> entry (e.g. star, tags, etc). The only difference is that it doesn't show the
> URL, and shows "Switch to Tab" instead.

Simply put it's because we're sitting on the already bookmarked page in our test that we don't see the decorations.
Aaron--I think after looking into this before, we decided there were no changes to Switch to Tab that should have affected this test.  The star not appearing is a bug.  

So, we decided to keep the failure as an actual failure until Bug 575609 is fixed.  In other words, the test isn't broken, it's failing as designed.  We shouldn't work around those.  :)

If I misunderstood, and you're saying current functionality is correct, please let me know.
Ah, if they are supposed to be there than this test should be fine until that Bug 575609 is fixed.
(Reporter)

Comment 12

8 years ago
(In reply to comment #11)
> Ah, if they are supposed to be there than this test should be fine until that
> Bug 575609 is fixed.

This is correct -- all work on this bug should be halted until the depends bug is fixed (as should be the standard policy).  Any work done on this bug could easily be invalidated by the fix for Bug 575609.  Please revisit this bug when bug 575609 is fixed.
In the future when tests get run on Tinderbox, we will have to disable such a test. So we could already follow this route and mark this test as skipped. We will see that in the results on brasstacks, because I have added those to the list. Joining that way we can limit our number of failed tests while we still know what has to be fixed.
Created attachment 457965 [details] [diff] [review]
Skip testStarInAutocomplete due to bug 575609

Per conversation with hskupin, putting in a skip for this test so it will be Tinderbox-compatible.  When this bug is resolved, the skip should be removed.
Attachment #457965 - Flags: review?(hskupin)
Created attachment 457966 [details] [diff] [review]
(respin) Skip testStarInAutocomplete [checked-in]

Oops, forgot the trailing semi on the last line.  Minor, but respun the patch to include.
Attachment #457965 - Attachment is obsolete: true
Attachment #457966 - Flags: review?(hskupin)
Attachment #457965 - Flags: review?(hskupin)
Attachment #457966 - Flags: review?(hskupin) → review+
Comment on attachment 457966 [details] [diff] [review]
(respin) Skip testStarInAutocomplete [checked-in]

Skip test patch has been landed on default:
http://hg.mozilla.org/qa/mozmill-tests/rev/6fd45a9757e0
Attachment #457966 - Attachment description: (respin) Skip testStarInAutocomplete due to bug 575609 → (respin) Skip testStarInAutocomplete [checked-in]
Geo, can you please enable this test again now that bug 575609 has been fixed?
Restored the test:

http://hg.mozilla.org/qa/mozmill-tests/rev/138569a906ea

The test appears to be broken.  I can visibly see the bookmark icon is set while the test is running, but it's failing the assert for it.  

I suspect something in the testToolbarAPI.js isn't matching the current setup, but I don't understand all the XPathing that's happening in there.  Will sync up with :whimboo tomorrow to fix.
You should check if the attribute has been changed:

> {isItemBookmarked: richlistItem.getNode().getAttribute('type') == 'bookmark'});

Eventually it would be a good idea to move this code into the autoCompleteResults class for better maintainability.
Created attachment 470873 [details] [diff] [review]
Fix bookmark type comparison for FF4

This fixes the actual test breakage for Firefox 4.  There's an additional error in PlaceAPI regarding re-importing the default bookmarks at the end of the test, but I'll file separately.
Attachment #470873 - Flags: review?(hskupin)
Comment on attachment 470873 [details] [diff] [review]
Fix bookmark type comparison for FF4

>-                      {isItemBookmarked: richlistItem.getNode().getAttribute('type') == 'bookmark'});
>+                      {isItemBookmarked: richlistItem.getNode().getAttribute('type') == 'action bookmark'});

"action" is an additional flag and i'm not sure if it will be present all the time. I feel we should better check for indexOf('bookmark') != -1.
Attachment #470873 - Flags: review?(hskupin) → review-
Created attachment 470953 [details] [diff] [review]
Make bookmark type comparison soft for FF4

Made it a soft comparison per Henrik's comment in case flags change.
Attachment #470953 - Flags: review?(hskupin)
Comment on attachment 470953 [details] [diff] [review]
Make bookmark type comparison soft for FF4

r=me. Looking forward in using assert() here which will reduce the line length a lot. Check it in as soon as you can. Thanks.
Attachment #470953 - Flags: review?(hskupin) → review+
With the patch applied I am getting an unexpected failure in the teardownModule
Seems like something is failing in restoreDefaultBookmarks but we aren't catching any errors for my output so it just fails in teardownModule
Landed on default as http://hg.mozilla.org/qa/mozmill-tests/rev/006305605c22

Aaron, yeah, what :whimboo said.  That's what I was referencing in comment 20.
Marking this fixed based on this error.  As mentioned above, bug 592411 is tracking the remaining teardownModule() error.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Mass move of Mozmill Test related project bugs to newly created components. You can filter out those emails by using "Mozmill-Tests-to-MozillaQA" as criteria.
Component: Location Bar → Mozmill Tests
Product: Firefox → Mozilla QA
QA Contact: location.bar → mozmill-tests
Version: Trunk → unspecified
You need to log in before you can comment on or make changes to this bug.