Closed Bug 1316154 Opened 3 years ago Closed 3 years ago

allow testing that an async event comes before or after other events

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: tbsaunde, Assigned: tbsaunde)

References

(Depends on 1 open bug)

Details

Attachments

(1 file)

There are cases where we want to test for several events, and while several
 may have a correct ordering some of the events can be fired in different
 orders WRT some but not all other events.  The only purpose of the
 orderChecker is to force that all preceeding checkers succeed before allowing
 any following checkers to match.
Comment on attachment 8808816 [details] [diff] [review]
allow testing that an async event comes before or after other events

Deferring to Yura.
Attachment #8808816 - Flags: review?(dbolter) → review?(yzenevich)
Comment on attachment 8808816 [details] [diff] [review]
allow testing that an async event comes before or after other events

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

thanks, just 1 question and a couple of nits.

::: accessible/tests/mochitest/events.js
@@ +541,5 @@
> +        if (eventSeq[idx] instanceof orderChecker && haveUnmatchedAsync) {
> +            break;
> +        }
> +
> +        if (!eventSeq[idx].wasCaught) {

Do we not need to ensure that it is indeed an expected async event (and maybe that it's not a todo) to set this flag?

@@ +569,5 @@
> +        } else {
> +          break;
> +        }
> +      }
> +      }

nit: indentation

@@ +614,5 @@
>      while (aEventSeq.idx < aEventSeq.length &&
>             (aEventSeq[aEventSeq.idx].unexpected ||
>              aEventSeq[aEventSeq.idx].todo ||
>              aEventSeq[aEventSeq.idx].async ||
> +               aEventSeq[aEventSeq.idx] instanceof orderChecker ||

nit: indentation

@@ +1702,5 @@
>  /**
> +  * event checker that forces preceeding async events to happen before this
> +  * checker.
> +  */
> +  function orderChecker()

Nit: indentation here and the jsdoc above

@@ +1704,5 @@
> +  * checker.
> +  */
> +  function orderChecker()
> +{
> +  this__proto__ = new invokerChecker(null, null, null, false);

missing '.' between this and __proto__
Attachment #8808816 - Flags: review?(yzenevich) → review+
Pushed by tsaunders@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb47917f2457
allow testing that an async event comes before or after other events r=yzen
> ::: accessible/tests/mochitest/events.js
> @@ +541,5 @@
> > +        if (eventSeq[idx] instanceof orderChecker && haveUnmatchedAsync) {
> > +            break;
> > +        }
> > +
> > +        if (!eventSeq[idx].wasCaught) {
> 
> Do we not need to ensure that it is indeed an expected async event (and
> maybe that it's not a todo) to set this flag?

requiring sync events to happen first too seems reasonable, maybe not todo I'm not sure about that though, and since we don't need it right now I don't see much reason to worry about it.
https://hg.mozilla.org/mozilla-central/rev/cb47917f2457
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Pushed by tsaunders@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a9358cf3143
workaround don't actually set the prototype of orderChecker
Comment on attachment 8808816 [details] [diff] [review]
allow testing that an async event comes before or after other events

test only dep of bug 1270916
Attachment #8808816 - Flags: approval-mozilla-aurora?
Comment on attachment 8808816 [details] [diff] [review]
allow testing that an async event comes before or after other events

Tests for big old e10s/a11y uplift, along with work from bug 1270916.
Attachment #8808816 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee: nobody → tbsaunde+mozbugs
Depends on: 1425371
You need to log in before you can comment on or make changes to this bug.