Closed Bug 1317164 Opened 3 years ago Closed 3 years ago

browser/components/customizableui/test/browser_967000_button_sync.js fails to run more than once in same browser session

Categories

(Firefox :: Toolbars and Customization, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 53
Tracking Status
firefox53 --- fixed

People

(Reporter: malayaleecoder, Assigned: malayaleecoder)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 2 obsolete files)

./mach mochitest browser/components/customizableui/test/browser_967000_button_sync.js --repeat 1 fails. This should not happen with our tests :)
Attached patch Bug1317164_v1.diff (obsolete) — Splinter Review
I am not sure whether this will break the purpose of the test. But it solves the multiple run failure issue.
Assignee: nobody → malayaleecoder
Status: NEW → ASSIGNED
Attachment #8810210 - Flags: review?(jmaher)
Blocks: 1316113
Summary: browser/components/customizableui/test/browser_967000_button_sync.js fails to run twice in same browser session → browser/components/customizableui/test/browser_967000_button_sync.js fails to more than once in same browser session
Summary: browser/components/customizableui/test/browser_967000_button_sync.js fails to more than once in same browser session → browser/components/customizableui/test/browser_967000_button_sync.js fails to run more than once in same browser session
Comment on attachment 8810210 [details] [diff] [review]
Bug1317164_v1.diff

redirecting to :markh, I am not familiar with this intentions of this test and possibly Mark will know or can help find someone.
Attachment #8810210 - Flags: review?(jmaher) → review?(markh)
Comment on attachment 8810210 [details] [diff] [review]
Bug1317164_v1.diff

Review of attachment 8810210 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/customizableui/test/browser_967000_button_sync.js
@@ +117,5 @@
>  }
>  
>  // When Sync is not setup.
>  add_task(() => openPrefsFromMenuPanel("PanelUI-remotetabs-setupsync", "synced-tabs"));
> +document.getElementById("PanelUI-remotetabs-setupsync").hidden = false;

So IIUC, this will show the panel as the test starts up - which if that is really the intent, it should probably be moved to the top of the test. However, I think it would be better to have openPrefsFromMenuPanel() do this to expectedPanelId() as it is finished.
Attachment #8810210 - Flags: review?(markh)
Attached patch Bug1317164_v2.diff (obsolete) — Splinter Review
Please have a look, I have incorporated the above said changes :)
Attachment #8810210 - Attachment is obsolete: true
Attachment #8812860 - Flags: review?(markh)
The above attached two screenshots were captured nearing the completion of the test runs. As you can notice, while running multiple times, there is a pop up of a SVG image (sync-illustration.svg) along with Sync options. Not sure how/why this is happening, just wanted to bring to notice.
Comment on attachment 8812860 [details] [diff] [review]
Bug1317164_v2.diff

I don't think that's quite right either, but I had a play and worked out what was :)
Attachment #8812860 - Attachment is obsolete: true
Attachment #8812860 - Flags: review?(markh)
Vishnu, can you make sure this runs fine on all of your tests locally and see if the code makes sense to you and r+ if so?
Attachment #8814310 - Flags: review+
Comment on attachment 8814310 [details]
Bug 1317164 - ensure browser_967000_button_sync.js cleans up after itself completely.

https://reviewboard.mozilla.org/r/95576/#review97180

It's all good
Attachment #8814310 - Flags: review?(malayaleecoder) → review+
Pushed by mhammond@skippinet.com.au:
https://hg.mozilla.org/integration/autoland/rev/8ea964e7ab59
ensure browser_967000_button_sync.js cleans up after itself completely. r=malayaleecoder
https://hg.mozilla.org/mozilla-central/rev/8ea964e7ab59
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
You need to log in before you can comment on or make changes to this bug.