Closed Bug 1245353 Opened 4 years ago Closed 4 years ago

tabs.reload has no unit tests

Categories

(WebExtensions :: Untriaged, defect)

defect
Not set

Tracking

(firefox48 fixed)

RESOLVED FIXED
mozilla48
Iteration:
48.2 - Apr 4
Tracking Status
firefox48 --- fixed

People

(Reporter: mattw, Assigned: mattw)

References

Details

Attachments

(1 file)

No description provided.
Flags: blocking-webextensions-
Flags: blocking-webextensions- → blocking-webextensions+
Iteration: --- → 47.2 - Feb 22
Iteration: 47.2 - Feb 22 → 47.3 - Mar 7
:mattw you moved it into the iteration, so assume you are working on it.
Assignee: nobody → mwein
Iteration: 47.3 - Mar 7 → 48.1 - Mar 21
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)
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)
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
Iteration: 48.1 - Mar 21 → ---
Iteration: --- → 48.2 - Apr 4
https://reviewboard.mozilla.org/r/40081/#review37655

I created the follow-up Bug 1257583 for testing `bypassCache`.
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/
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/28a35458f825
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.