Test coverage for webNavigation extension API

NEW
Assigned to

Status

()

Toolkit
WebExtensions: General
P3
normal
2 years ago
8 months ago

People

(Reporter: krizsa, Assigned: rpl)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [webNavigation] triaged)

Attachments

(1 obsolete attachment)

(Reporter)

Description

2 years ago
https://wiki.mozilla.org/Browser_Extensions#List_of_supported_APIs
(Reporter)

Updated

2 years ago
Blocks: 1185459
Created attachment 8689834 [details] [diff] [review]
patch

This just copies the test we have for the .jsm.
Attachment #8689834 - Flags: review?(kmaglione+bmo)
Comment on attachment 8689834 [details] [diff] [review]
patch

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

::: toolkit/components/extensions/test/mochitest/test_ext_webnavigation.html
@@ +55,5 @@
> +    browser.test.assertTrue(details.frameId !== undefined);
> +    browser.test.assertTrue(details.parentFrameId !== undefined);
> +  }
> +
> +  var listeners = {};

This doesn't appear to actually be used.

@@ +86,5 @@
> +  received = [];
> +  waitingEvent = event;
> +  waitingURL = url;
> +  dump(`RUN ${script}\n`);
> +  eval(script);

Can you make this a lambda rather than using eval?
Attachment #8689834 - Flags: review?(kmaglione+bmo) → review+
Component: Extension Compatibility → WebExtensions
Product: Firefox → Toolkit

Updated

a year ago
Flags: blocking-webextensions-

Updated

a year ago
Flags: blocking-webextensions- → blocking-webextensions+
Luca, it looks like there's a patch from billm that we could probably clean up and merge?

Also looking at code coverage: https://people.mozilla.org/~kmaglione/webextension-test-coverage/toolkit/components/extensions/ext-webNavigation.js.html there's a few small error conditions that need testing, should we make a seperate bug for those?
Assignee: nobody → lgreco
(Assignee)

Comment 4

a year ago
(In reply to Andy McKay [:andym] from comment #3)
> Luca, it looks like there's a patch from billm that we could probably clean
> up and merge?

That patch has already landed in the following changeset (and then tweaked during the implementation of the other webNavigation features):

- https://hg.mozilla.org/mozilla-central/rev/5af1998a1366
 
> Also looking at code coverage:
> https://people.mozilla.org/~kmaglione/webextension-test-coverage/toolkit/
> components/extensions/ext-webNavigation.js.html there's a few small error
> conditions that need testing, should we make a seperate bug for those?

I see, I'm going to look into the scenarios to which they correspond (and a brief look on the other changes that Bug 1190687 is going to introduce into the same file) and then file bugs with descriptions of the scenarios that are not yet covered.
(Assignee)

Comment 5

a year ago
Comment on attachment 8689834 [details] [diff] [review]
patch

Marking as obsolete this patch because it is already landed as:

- https://hg.mozilla.org/mozilla-central/rev/5af1998a1366
Attachment #8689834 - Attachment is obsolete: true

Updated

a year ago
Whiteboard: [webNavigation] triaged

Updated

a year ago
Flags: blocking-webextensions+ → blocking-webextensions-
(Assignee)

Updated

a year ago
Depends on: 1246125
(Assignee)

Updated

a year ago
Depends on: 1262794

Comment 6

8 months ago
So close, 97.26% https://people.mozilla.org/~kmaglione/webextension-test-coverage/toolkit/components/extensions/ext-webNavigation.js.html
Component: WebExtensions: Untriaged → WebExtensions: General
Flags: blocking-webextensions-
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.