Test coverage for |extension| extension API

RESOLVED FIXED in Firefox 48

Status

RESOLVED FIXED
3 years ago
4 months ago

People

(Reporter: gkrizsanits, Assigned: bsilverberg)

Tracking

unspecified
mozilla48
Bug Flags:
blocking-webextensions +

Firefox Tracking Flags

(firefox48 fixed)

Details

(Whiteboard: [testing] triaged)

Attachments

(1 attachment)

Comment hidden (empty)
(Reporter)

Updated

3 years ago
Blocks: 1185459
This is still missing coverage for:

* The |getViews| API method with any properties object at all, and in particular with |type| or |windowId| properties.


https://people.mozilla.org/~kmaglione/webextension-test-coverage/toolkit/components/extensions/ext-extension.js.html

Updated

3 years ago
Component: Extension Compatibility → WebExtensions
Product: Firefox → Toolkit

Updated

3 years ago
Flags: blocking-webextensions-

Updated

3 years ago
Flags: blocking-webextensions- → blocking-webextensions+

Updated

3 years ago
Assignee: nobody → bob.silverberg

Updated

3 years ago
Whiteboard: [testing] triaged
(Assignee)

Updated

3 years ago
Iteration: --- → 48.3 - Apr 18
(Assignee)

Comment 2

3 years ago
According to [1], this also now needs coverage for:

* The |inIncognitoContext| property.

[1] https://people.mozilla.org/~kmaglione/webextension-test-coverage/toolkit/components/extensions/ext-extension.js.html
Status: NEW → ASSIGNED
(Assignee)

Comment 3

3 years ago
Created attachment 8739007 [details]
MozReview Request: Bug 1190322 - Test coverage for |extension| extension API, r?kmag

Add coverage for:

* The |getViews| API method with a |type| property.
* The |getViews| API method with a |windowId| property.
* The |inIncognitoContext| property.

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

Comment 4

3 years ago
https://reviewboard.mozilla.org/r/45019/#review41523

::: toolkit/components/extensions/test/mochitest/test_ext_extension.html:50
(Diff revision 1)
> +  yield extension.startup();
> +  yield extension.awaitFinish("inIncognitoContext");
> +  yield extension.unload();
> +});
> +
> +add_task(function* test_in_incognito_context_true() {

This is my attempt to write a test that would expect `inIncognitoContext` to be `true`. The test proceeds as expected, but `inIncognitoContext` is not `true` because the code is not running in a private window. I have a couple of questions about this:

1. What is the case in which we would expect `inIncognitoContext` to return `true`.
2. How can I create that scenario in the context of a test? I've tried opening private windows and tabs, but I haven't been able to get it to work. Is there a way to open a private window via the `window.open` command that I am using on line 89?
Comment on attachment 8739007 [details]
MozReview Request: Bug 1190322 - Test coverage for |extension| extension API, r?kmag

https://reviewboard.mozilla.org/r/45019/#review41601

Please file a different bug for the `isIncognitoContext` tests. The others look fine.

::: browser/components/extensions/test/browser/browser_ext_getViews.js:117
(Diff revision 1)
>    function* openTab(winId) {
>      extension.sendMessage("background-open-tab", winId);
>      yield extension.awaitMessage("tab-ready");
>    }
>  
> -  function* checkViews(kind, tabCount, popupCount) {
> +  function* checkViews(kind, tabCount, popupCount, kindCount, windowId, windowCount = 0) {

Please add `undefined` as a default value for `windowId` to make it clear that it's optional.
Attachment #8739007 - Flags: review?(kmaglione+bmo) → review+
(In reply to Bob Silverberg [:bsilverberg] from comment #4)
> 1. What is the case in which we would expect `inIncognitoContext` to return
> `true`.

That's a question without a clear answer. I'd expect it to return true for:

 - Content scripts running in tabs in private windows.
 - Web extension pages loaded into tabs in private windows.
 - Web extension pages loaded into popups in private windows.

I'm not sure, though, which of those currently work, or what Chrome does for
any of them.

> 2. How can I create that scenario in the context of a test? I've tried
> opening private windows and tabs, but I haven't been able to get it to work.
> Is there a way to open a private window via the `window.open` command that I
> am using on line 89?

window.open({incognito: true});
(Assignee)

Comment 7

3 years ago
Comment on attachment 8739007 [details]
MozReview Request: Bug 1190322 - Test coverage for |extension| extension API, r?kmag

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

Updated

3 years ago
Blocks: 1263167
(Assignee)

Updated

3 years ago
No longer blocks: 1263167
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
Needs rebasing.
Keywords: checkin-needed
(Assignee)

Comment 11

3 years ago
Comment on attachment 8739007 [details]
MozReview Request: Bug 1190322 - Test coverage for |extension| extension API, r?kmag

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

Comment 12

3 years ago
Ok, rebased.
Keywords: checkin-needed

Comment 15

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

Updated

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