Test coverage for webNavigation extension API

RESOLVED FIXED

Status

P3
normal
RESOLVED FIXED
4 years ago
8 months ago

People

(Reporter: gkrizsanits, Assigned: rpl)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [webNavigation] triaged)

Attachments

(1 obsolete attachment)

(Reporter)

Updated

4 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

3 years ago
Flags: blocking-webextensions-

Updated

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

Comment 3

3 years ago
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

3 years 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

3 years 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

3 years ago
Whiteboard: [webNavigation] triaged

Updated

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

Updated

3 years ago
Depends on: 1246125
(Assignee)

Updated

3 years ago
Depends on: 1262794

Comment 6

2 years 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

Comment 7

2 years ago
Currently at 99.16% https://coveralls.io/builds/12804880/source?filename=toolkit%2Fcomponents%2Fextensions%2Fext-webNavigation.js and all the dependencies done. If we aim for 100%+ one day, we can revisit this.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED

Updated

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