Closed Bug 1317019 Opened 6 years ago Closed 6 years ago

Fix JS Debugger code coverage collection for when source file has no methods covered.

Categories

(Testing :: Code Coverage, defect)

defect
Not set
normal

Tracking

(firefox54 fixed)

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: sparky, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

This bug occurs in "CoverageUtils.jsm" and because there is no condition that checks if a given source file has methods that were touched/used. "test_PrivateTemp.js" is one of the tests that cause this problem to occur.
Here's an example for before and after applying the patch: https://pastebin.mozilla.org/8927404 .
Comment on attachment 8810021 [details]
Bug 1317019 - Fix object iteration and check if scriptName exists.

https://reviewboard.mozilla.org/r/92486/#review92850
Attachment #8810021 - Flags: review?(cmanchester) → review+
Comment on attachment 8810021 [details]
Bug 1317019 - Fix object iteration and check if scriptName exists.

https://reviewboard.mozilla.org/r/92486/#review110552

::: testing/modules/CoverageUtils.jsm:169
(Diff revision 2)
> + */
> +Object.prototype[Symbol.iterator] = function * () {
> +  for (var [key, value] of Object.entries(this)) {
> +    yield [key, value];
> +  }
> +};

do we have a danger of accidentally using this in the future while adding code to this module?  Not too concerning but I would like your thoughts on this
Attachment #8810021 - Flags: review?(jmaher) → review+
Comment on attachment 8810021 [details]
Bug 1317019 - Fix object iteration and check if scriptName exists.

https://reviewboard.mozilla.org/r/92486/#review110552

> do we have a danger of accidentally using this in the future while adding code to this module?  Not too concerning but I would like your thoughts on this

I was wondering the same thing and I think that it could be accidently used, but that usage wouldn't be problematic.

This simply implements an iterator for iterating over objects that are within objects (by objects, I mean variables created using '{}') so it shouldn't affect anything outside of this. I think that at worst, if it is used accidently, it would help with iterating over an object within another object. So, the problem we encounter seems like more of a bonus for me - especially since it makes the code more clear, & elegant.

In my opinon, even if this module is included in another file and used completely, it'll do the same thing that I mention above. So, I'm actually having a hard time trying to think of where this could actually be problematic and break something. Which, I believe, is a good thing. :)
Pushed by jmaher@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ddd06f373719
Fix object iteration and check if scriptName exists. r=chmanchester,jmaher
https://hg.mozilla.org/mozilla-central/rev/ddd06f373719
Status: UNCONFIRMED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Depends on: 1435040
You need to log in before you can comment on or make changes to this bug.