Intermittent dom/base/test/test_intersectionobservers.html | triggers only once if observed multiple times (and does not crash when collected) [observe] - got +0, expected 1

RESOLVED FIXED in Firefox 54

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: intermittent-bug-filer, Assigned: tschneider)

Tracking

({intermittent-failure})

unspecified
mozilla55
intermittent-failure
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox53 unaffected, firefox54 fixed, firefox55 fixed)

Details

(Whiteboard: [stockwell fixed])

Attachments

(2 attachments, 1 obsolete attachment)

Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
ni myself to investigate a bit more
Flags: needinfo?(jmaher)
Whiteboard: [stockwell needswork]
Flags: needinfo?(jmaher)
this was related to bug 1321865, it was backed out as we had a ~perma fail in bug 1313972.
Whiteboard: [stockwell needswork] → [stockwell disabled]
Can this test be rewritten this way to remove the race?

      it('triggers only once if observed multiple times (and does not crash when collected)', function(done) {
        var spy = sinon.spy();
        io = new IntersectionObserver(spy, {root: rootEl});
        io.observe(targetEl1);
        io.observe(targetEl1);
        io.observe(targetEl1);

        spy.waitForNotification(function() {
          callDelayed(function () {
            expect(spy.callCount).to.be(1);
            done();
          }, ASYNC_TIMEOUT);
        }, 0);

      });
Flags: needinfo?(tschneider)
(Assignee)

Comment 9

2 years ago
Yes, that would work.
Flags: needinfo?(tschneider)
(Assignee)

Comment 10

2 years ago
Created attachment 8858436 [details] [diff] [review]
Fix intermittent test failure

Applies recommended change from Comment 8 plus removes unnecessary delay arguments.
(Assignee)

Updated

2 years ago
Attachment #8858436 - Flags: review?(bugs)
Comment on attachment 8858436 [details] [diff] [review]
Fix intermittent test failure


>@@ -918,10 +918,12 @@ limitations under the License.
>         io.observe(targetEl1);
>         io.observe(targetEl1);
> 
>-        callDelayed(function () {
>-          expect(spy.callCount).to.be(1);
>-          done();
>-        }, ASYNC_TIMEOUT * 3);
>+        spy.waitForNotification(function() {
>+          callDelayed(function () {
>+            expect(spy.callCount).to.be(1);
>+            done();
>+          }, ASYNC_TIMEOUT);
>+        }, 0);

Lose the 0 here if that's your new convention.

r+ with nits fixed and better commit message. Maybe:

Wait 300ms after the first IntersectionObserver notification to prevent intermittent test failure.
Attachment #8858436 - Flags: review?(bugs) → review+
(Assignee)

Comment 13

2 years ago
Created attachment 8858491 [details] [diff] [review]
Wait 300ms after the first IntersectionObserver notification to prevent intermittent test failure.

Nits.
Attachment #8858436 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 14

2 years ago
Pushed by philringnalda@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/940919c5af90
Wait 300ms after the first IntersectionObserver notification to prevent intermittent test failure, r=jet
Keywords: checkin-needed
Assignee: nobody → tschneider
https://hg.mozilla.org/mozilla-central/rev/940919c5af90
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
(Assignee)

Comment 16

2 years ago
Comment on attachment 8858491 [details] [diff] [review]
Wait 300ms after the first IntersectionObserver notification to prevent intermittent test failure.

Approval Request Comment
[Feature/Bug causing the regression]: 1321865
[User impact if declined]: No test coverage
[Is this code covered by automated tests?]: It is one
[Has the fix been verified in Nightly?]: Yes
[Is the change risky?]: No
Attachment #8858491 - Flags: approval-mozilla-beta?
(Assignee)

Comment 17

2 years ago
Comment on attachment 8858491 [details] [diff] [review]
Wait 300ms after the first IntersectionObserver notification to prevent intermittent test failure.

not part of the build
Attachment #8858491 - Flags: approval-mozilla-beta?
(Assignee)

Comment 18

2 years ago
Created attachment 8864630 [details] [diff] [review]
Patch for beta uplift
(Assignee)

Updated

2 years ago
Blocks: 1362168
(Assignee)

Updated

2 years ago
Whiteboard: [stockwell disabled] → [stockwell disabled] [checkin-needed-beta]
status-firefox53: --- → unaffected
status-firefox54: --- → affected
status-firefox-esr52: --- → unaffected

Comment 19

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/b0a7956aa594
status-firefox54: affected → fixed
Flags: in-testsuite+
Whiteboard: [stockwell disabled] [checkin-needed-beta] → [stockwell fixed]
Comment hidden (Intermittent Failures Robot)
You need to log in before you can comment on or make changes to this bug.