Closed Bug 760082 Opened 9 years ago Closed 9 years ago

toolkit/components/satchel/test/test_form_autocomplete_with_list.html ran additional tests after finish() was called

Categories

(Toolkit :: Autocomplete, defect)

defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: mats, Assigned: mats)

Details

Attachments

(1 file)

Fallout from the diagnostic patch in bug 677964.
Comment on attachment 628694 [details] [diff] [review]
Move SimpleTest.finish() after the last test is done.  Use info() for log messages that aren't tests.

That test lost me three times before I even got down to the patch.
Attachment #628694 - Flags: review?(philringnalda) → review?(dolske)
Comment on attachment 628694 [details] [diff] [review]
Move SimpleTest.finish() after the last test is done.  Use info() for log messages that aren't tests.

Fine with me if it fixes the problem. But... Are you sure this was the issue your diagnostic caught?

I'd have expected the input event to be fired asynchronously. And so the last bit of the case-400 should run before the input event fires. Is the event actually firing synchronously? I bet Mounir will know, his code and test. :)
Attachment #628694 - Flags: review?(dolske) → review?(mounir)
(In reply to Justin Dolske [:Dolske] from comment #3)
> Comment on attachment 628694 [details] [diff] [review]
> Move SimpleTest.finish() after the last test is done.  Use info() for log
> messages that aren't tests.
> 
> Fine with me if it fixes the problem. But... Are you sure this was the issue
> your diagnostic caught?
> 
> I'd have expected the input event to be fired asynchronously. And so the
> last bit of the case-400 should run before the input event fires. Is the
> event actually firing synchronously? I bet Mounir will know, his code and
> test. :)

This is actually not my code nor my test. That was David Zbarsky's. However, I can have a look.
Comment on attachment 628694 [details] [diff] [review]
Move SimpleTest.finish() after the last test is done.  Use info() for log messages that aren't tests.

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

The best thing to do here to be on the safe side would be:
    case 400:
      // Check that the input event is fired.
      input.addEventListener("input", function(event) {
        input.removeEventListener("input", arguments.callee, false);
        ok(true, "oninput should have been received");
        ok(event.bubbles, "input event should bubble");
        ok(event.cancelable, "input event should be cancelable");

        checkForm("Google");
        SimpleTest.finish();
      }, false);

      doKey("down");
      checkForm("");
      doKey("return");

r=me with that.
Attachment #628694 - Flags: review?(mounir) → review+
OK, landed with that change:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac3642f73f24
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/ac3642f73f24
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.