tabs.reload has no unit tests

RESOLVED FIXED in Firefox 48

Status

RESOLVED FIXED
3 years ago
8 months ago

People

(Reporter: mattw, Assigned: mattw)

Tracking

unspecified
mozilla48
Bug Flags:
blocking-webextensions +

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(1 attachment)

Comment hidden (empty)

Updated

3 years ago
Flags: blocking-webextensions-

Updated

3 years ago
Flags: blocking-webextensions- → blocking-webextensions+
(Assignee)

Updated

3 years ago
Iteration: --- → 47.2 - Feb 22
(Assignee)

Updated

3 years ago
Iteration: 47.2 - Feb 22 → 47.3 - Mar 7

Comment 1

3 years ago
:mattw you moved it into the iteration, so assume you are working on it.
Assignee: nobody → mwein
(Assignee)

Updated

3 years ago
Iteration: 47.3 - Mar 7 → 48.1 - Mar 21
(Assignee)

Comment 2

3 years ago
Created attachment 8730684 [details]
MozReview Request: Bug 1245353 Add tests for tabs.reload r?kmag

Review commit: https://reviewboard.mozilla.org/r/40081/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/40081/
Attachment #8730684 - Flags: review?(kmaglione+bmo)
(Assignee)

Comment 3

3 years ago
Do you think this is an adequate test or should we also confirm that the page actually reloads? Do you know if we can do that?
Comment on attachment 8730684 [details]
MozReview Request: Bug 1245353 Add tests for tabs.reload r?kmag

https://reviewboard.mozilla.org/r/40081/#review36637

It's a start, but we need to do something to check that the tabs actually reload. Loading a page from the extension and having it send a message via `browser.runtime` should work for a start.

We should really try to test `bypassCache` somehow, though. Maybe file a follow-up bug?

::: browser/components/extensions/test/browser/browser_ext_tabs_reload.js:18
(Diff revision 1)
> +        browser.tabs.query({currentWindow: true, active: true}).then(tabs => {
> +          browser.tabs.reload(tabs[0].id, {bypassCache: true}).then(() => {
> +            browser.test.notifyPass("tabs.reload");
> +          });
> +        });
> +      });

Generally you'd phrase this as a promise chain, e.g.,

    browser.tabs.reload().then(() => {
      return browser.tabs.query({currentWindow: true, active: true});
    }).then(tabs => {
      return browser.tabs.reload(tabs[0].id, {bypassCache: true});
    }).then(() => {
      browser.test.notifyPass("tabs.reload");
    });
Attachment #8730684 - Flags: review?(kmaglione+bmo)
(Assignee)

Comment 5

3 years ago
Comment on attachment 8730684 [details]
MozReview Request: Bug 1245353 Add tests for tabs.reload r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40081/diff/1-2/
Attachment #8730684 - Flags: review?(kmaglione+bmo)
(Assignee)

Comment 6

3 years ago
Follow up bug created for testing `bypassCache`: https://bugzilla.mozilla.org/show_bug.cgi?id=1257583.
Attachment #8730684 - Flags: review?(kmaglione+bmo) → review+
Comment on attachment 8730684 [details]
MozReview Request: Bug 1245353 Add tests for tabs.reload r?kmag

https://reviewboard.mozilla.org/r/40081/#review37655
(Assignee)

Updated

3 years ago
Iteration: 48.1 - Mar 21 → ---
(Assignee)

Updated

3 years ago
Iteration: --- → 48.2 - Apr 4
(Assignee)

Comment 8

3 years ago
https://reviewboard.mozilla.org/r/40081/#review37655

I created the follow-up Bug 1257583 for testing `bypassCache`.
(Assignee)

Comment 9

3 years ago
Comment on attachment 8730684 [details]
MozReview Request: Bug 1245353 Add tests for tabs.reload r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40081/diff/2-3/
(Assignee)

Updated

3 years ago
Keywords: checkin-needed

Comment 11

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/28a35458f825
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48

Updated

8 months ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.