1551275 - Refactor weak map jsapi tests

Status: RESOLVED FIXED

(Core :: JavaScript: GC, task, P3)

Description

[Jon Coppeard (:jonco)]

In preparation for investigating the weak map marking assertions in bug 1514421 I thought I'd refactor the weak map tests a little. This should make it easier to spot if there are any cases we miss.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: Bug 1551275 - Refactor jsapi weak map tests r=sfink

This removes a bunch of repeated code and hopefully makes it easier to see what we're testing. When marking two things the same color this now checks both orders (e.g. key before map, map before key). I also enabled the GC weak map marking verification to check that doesn't fail any of these cases.

The tests for JS WeakMap and internal weakmaps are slightly different because I wanted to cover all existing test cases without making things too complicated. This means we don't test marking the key and delegate different colors for the former.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b993d1489eee
Refactor jsapi weak map tests r=sfink

Comment 3

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/b993d1489eee

Comment 4

[Jon Coppeard (:jonco)]

Comment on attachment 9064515 [details]
Bug 1551275 - Refactor jsapi weak map tests r=sfink

Beta/Release Uplift Approval Request

• User impact if declined: Required for uplift of bug 1551907.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Test code changes only.
• String changes made/needed:

Comment 5

[Jon Coppeard (:jonco)]

Comment on attachment 9064515 [details]
Bug 1551275 - Refactor jsapi weak map tests r=sfink

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Required for uplift of bug 1551907.
• User impact if declined: None.
• Fix Landed on Version: 69
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Test code changes only.
• String or UUID changes made by this patch:

Comment 6

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 9064515 [details]
Bug 1551275 - Refactor jsapi weak map tests r=sfink

approved for 68.0b5

Comment 7

[Oana Pop-Rus]

https://hg.mozilla.org/releases/mozilla-beta/rev/e408eb60134b

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9064515 [details]
Bug 1551275 - Refactor jsapi weak map tests r=sfink

test-only prereq patch for bug 1551907. Approved for 60.8esr.

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

There are conflicts for the test file. Please provide a patch for ESR60.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9064515 [details]
Bug 1551275 - Refactor jsapi weak map tests r=sfink

Not needed on ESR60 after all.