Closed
Bug 573587
Opened 15 years ago
Closed 14 years ago
[mozmill] Assertion failure in testSuggestHistoryBookmarks.js
Categories
(Mozilla QA Graveyard :: Mozmill Tests, defect)
Mozilla QA Graveyard
Mozmill Tests
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: u279076, Assigned: gmealer)
References
()
Details
(Keywords: regression, Whiteboard: [mozmill-test-failure])
Attachments
(3 files, 1 obsolete file)
(deleted),
patch
|
whimboo
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
whimboo
:
review-
|
Details | Diff | Splinter Review |
(deleted),
patch
|
whimboo
:
review+
|
Details | Diff | Splinter Review |
MODULE: /firefox/testAwesomeBar/testSuggestHistoryBookmarks.js
TEST: testStarInAutocomplete (107)
FAIL: Controller.assertJS("subject.isItemBookmarked == true")
BRANCH: default, 1.9.1
PLATFORMS: All
Whiteboard: [mozmill-test-failure]
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.
Comment 2•15 years ago
|
||
(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
See bug 573618 for the 1.9.1 issue.
Assignee | ||
Comment 4•14 years ago
|
||
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.
Comment 5•14 years ago
|
||
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
Comment 6•14 years ago
|
||
(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.
Comment 7•14 years ago
|
||
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.
Comment 8•14 years ago
|
||
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.
Comment 9•14 years ago
|
||
(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.
Assignee | ||
Comment 10•14 years ago
|
||
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.
Comment 11•14 years ago
|
||
Ah, if they are supposed to be there than this test should be fine until that Bug 575609 is fixed.
Reporter | ||
Comment 12•14 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.
Comment 13•14 years ago
|
||
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.
Assignee | ||
Comment 14•14 years ago
|
||
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)
Assignee | ||
Comment 15•14 years ago
|
||
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)
Updated•14 years ago
|
Attachment #457966 -
Flags: review?(hskupin) → review+
Comment 16•14 years ago
|
||
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]
Comment 17•14 years ago
|
||
Geo, can you please enable this test again now that bug 575609 has been fixed?
Assignee | ||
Comment 18•14 years ago
|
||
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.
Comment 19•14 years ago
|
||
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.
Assignee | ||
Comment 20•14 years ago
|
||
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 21•14 years ago
|
||
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-
Assignee | ||
Comment 22•14 years ago
|
||
Made it a soft comparison per Henrik's comment in case flags change.
Attachment #470953 -
Flags: review?(hskupin)
Comment 23•14 years ago
|
||
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+
Comment 24•14 years ago
|
||
With the patch applied I am getting an unexpected failure in the teardownModule
Comment 25•14 years ago
|
||
Seems like something is failing in restoreDefaultBookmarks but we aren't catching any errors for my output so it just fails in teardownModule
Comment 26•14 years ago
|
||
This is bug 592411.
Assignee | ||
Comment 27•14 years ago
|
||
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.
Assignee | ||
Comment 28•14 years ago
|
||
Marking this fixed based on this error. As mentioned above, bug 592411 is tracking the remaining teardownModule() error.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 29•14 years ago
|
||
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
Updated•5 years ago
|
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•