Closed Bug 1067509 Opened 7 years ago Closed 7 years ago

[AccessFu] Refactor content tests.

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35
Tracking Status
firefox34 --- fixed
firefox35 --- fixed

People

(Reporter: eeejay, Assigned: eeejay)

References

Details

Attachments

(1 file, 5 obsolete files)

No description provided.
Attachment #8489531 - Flags: feedback?(yzenevich)
Attachment #8489531 - Attachment is obsolete: true
Attachment #8489531 - Flags: feedback?(yzenevich)
Attachment #8490192 - Flags: review?(yzenevich)
Attachment #8490192 - Attachment is obsolete: true
Attachment #8490192 - Flags: review?(yzenevich)
Attachment #8490333 - Flags: review?(yzenevich)
Attachment #8490333 - Attachment is obsolete: true
Attachment #8490333 - Flags: review?(yzenevich)
Attachment #8491002 - Flags: review?(yzenevich)
Got these errors with the patch:

The following tests failed:
3698 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/tests/mochitest/jsat/test_content_text.html | 0 [ eventType [ expected 131072 got 8192 ] fromIndex [ expected 53 got 59 ] -- undefined ] -- after {"name":"AccessFu:MoveCaret","json":{"direction":"Previous","granularity":2}} (20) (android) - expected PASS
3699 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/tests/mochitest/jsat/test_content_text.html | Expected something but got nothing -- after {"name":"AccessFu:MoveCursor","json":{"action":"moveNext","rule":"Simple","inputType":"gesture","origin":"top"}} (21) (b2g) - expected PASS
3700 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/tests/mochitest/jsat/test_content_text.html | 0 [ eventType [ expected 32768 got 131072 ] -- undefined ] -- after {"name":"AccessFu:MoveCursor","json":{"action":"moveNext","rule":"Simple","inputType":"gesture","origin":"top"}} (21) (android) - expected PASS
3701 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/tests/mochitest/jsat/test_content_text.html | 0 [ eventType [ expected 16384 got 32768 ] text [ 0 [ expected "navigating" got "So we don't get dessert?" ] -- undefined ] addedCount [ expected 10 got undefined ] -- undefined ] -- after {"name":"AccessFu:MoveCursor","json":{"action":"moveNext","rule":"Simple","inputType":"gesture","origin":"top"}} (21) (android) - expected PASS
Comment on attachment 8491002 [details] [diff] [review]
Refactor jsat content test runner.

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

first round, ill go over it again once the tests pass.

::: accessible/jsat/EventManager.jsm
@@ +198,5 @@
>          // We could get a caret move in an accessible that is not focused,
>          // it doesn't mean we are not on any editable accessible. just not
>          // on this one..
> +        let state = Utils.getState(acc);
> +        if (state.contains(States.FOCUSED)) {

Curious if we need to check for a read only state as well

@@ +203,3 @@
>            this._setEditingMode(aEvent, caretOffset);
> +          if (state.contains(States.EDITABLE)) {
> +            this.present(Presentation.textSelectionChanged(acc.getText(0,-1),

Nit space between 0 and -1

@@ +203,4 @@
>            this._setEditingMode(aEvent, caretOffset);
> +          if (state.contains(States.EDITABLE)) {
> +            this.present(Presentation.textSelectionChanged(acc.getText(0,-1),
> +             caretOffset, caretOffset, 0, 0, aEvent.isFromUserInput));

Nit: expected 2 spaces.

@@ +258,5 @@
>            this.contentControl.autoMove(acc);
>         }
> +
> +       if (this.inTest) {
> +        this.sendMsgFunc("AccessFu:Focused", {});

Do we need an empty {} here?

::: accessible/tests/mochitest/jsat/jsatcommon.js
@@ +495,5 @@
> +};
> +
> +ExpectedMessage.prototype.ignore = function(aMessage) {
> +  return aMessage.name !== this.name;
> +}

Missing ;

@@ +508,5 @@
> +    this.json.android = aAndroid;
> +  }
> +}
> +
> +ExpectedPresent.prototype = new ExpectedMessage();

What about:
ExpectedPresent.prototype = Object.create(ExpectedMessage.prototype);

@@ +513,5 @@
> +
> +ExpectedPresent.prototype.is = function(aReceived, aInfo) {
> +  var received = this.extract_presenters(aReceived);
> +
> +  if (!this.options.no_b2g) {

This would be nice if wrapped in a function (since b2g and android are so similar).

@@ +528,5 @@
> +    SimpleTest[androidCheckFunc].apply(
> +      SimpleTest, this.lazyCompare(received.android, this.json.android, aInfo +
> +        ' (android)'));
> +  }
> +}

Nit: ;

@@ +532,5 @@
> +}
> +
> +ExpectedPresent.prototype.extract_presenters = function(aReceived) {
> +  var received = { count: 0 };
> +  for (var i in aReceived) {

var {variable} of aReceived

@@ +552,5 @@
> +  return received.count === 0 ||
> +    (received.visual && received.visual.eventType === 'viewport-change') ||
> +    (received.android &&
> +      received.android[0].eventType === AndroidEvent.VIEW_SCROLLED);
> +}

Nit ;

@@ +563,5 @@
> +    eventType: 0x8000, // VIEW_ACCESSIBILITY_FOCUSED
> +  }], aOptions);
> +}
> +
> +ExpectedCursorChange.prototype = new ExpectedPresent();

What about:
ExpectedCursorChange.prototype = Object.create(ExpectedPresent.prototype);

@@ +579,5 @@
> +  // bug 980509
> +  this.options.b2g_todo = true;
> +}
> +
> +ExpectedCursorTextChange.prototype = new ExpectedCursorChange();

ExpectedCursorTextChange.prototype = Object.create(ExpectedCursorChange.prototype); ?

And likewise below.
Attachment #8491002 - Flags: review?(yzenevich)
Attachment #8491002 - Attachment is obsolete: true
Attachment #8491661 - Flags: review?(yzenevich)
Attachment #8491661 - Attachment is obsolete: true
Attachment #8491661 - Flags: review?(yzenevich)
Attachment #8491751 - Flags: review?(yzenevich)
Comment on attachment 8491751 [details] [diff] [review]
Refactor jsat content test runner.

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

awesome! r=me

::: accessible/tests/mochitest/jsat/jsatcommon.js
@@ +520,5 @@
> +
> +ExpectedPresent.prototype.is = function(aReceived, aInfo) {
> +  var received = this.extract_presenters(aReceived);
> +
> +  for (var presenter of ['b2g', 'android']) {

Nice!
Attachment #8491751 - Flags: review?(yzenevich) → review+
This did land on mozilla-central, though the bug didn't get marked. Sorry for any confusion.
https://hg.mozilla.org/mozilla-central/rev/a02351de6e5b
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.