Closed Bug 415730 Opened 12 years ago Closed 8 years ago

convert existing API tests into mochitests

Categories

(Core :: Disability Access APIs, defect)

x86
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: bernd_mozilla, Assigned: MarcoZ)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

http://www.mozilla.org/quality/embed/plans/accessibility/HTMLAccessibility.html

contains tests which are by its nature very similar to mochitests. We should convert them, bring them into the tree and make them part of the tinderbox testing.

alexander surkov in bug 410052
"We can add mochitest but we can't enable this because some another mochitests
become broken with enabled accessibility. I think we'll take care of this after
Firefox3."

There needs to be a bug reference for this to get this solved before.
Assigning this to myself since this is going to be my baby. As stated in bug 410052, I will find a way to have ally Mochitests run after all others are run, so that when a11y gets enabled, it doesn#t interfere with Mochitests that depend on being run without.
Status: NEW → ASSIGNED
Assignee: aaronleventhal → marco.zehe
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
remember about bug 410052 to add test for it
(In reply to comment #2)
> Assigning this to myself since this is going to be my baby. As stated in bug
> 410052, I will find a way to have ally Mochitests run after all others are run,
> so that when a11y gets enabled, it doesn#t interfere with Mochitests that
> depend on being run without.
> 

Not sure it's correct way to run our mochitests after all tests because mochitests shouldn't fail in anyway when ally is enabled.
I think we should do something similar to the way we're running browser-chrome and chrome tests. Provide a separate option to runtests.pl|y (--a11y ?) that runs tests from a separate location. We might want to keep them isolated.

Also, and this is probably a separate bug, but Martijn suggested last week that we should consider setting up a machine/profile to run mochitests with accessibility turned on to see what breaks.
(In reply to comment #6)
> I think we should do something similar to the way we're running browser-chrome
> and chrome tests. Provide a separate option to runtests.pl|y (--a11y ?) that
> runs tests from a separate location. We might want to keep them isolated.

Ok, I'll try this.

> Also, and this is probably a separate bug, but Martijn suggested last week that
> we should consider setting up a machine/profile to run mochitests with
> accessibility turned on to see what breaks.
> 

It would be excellent.
Depends on: 416553
Blocks: a11ytestdev
the columnHeader is bug 417797
the rwoHeader    is bug 417791
Just to avoid duplicates, I will write a complete set for nsIAccessibleTable.
Attachment #303769 - Flags: review?
Attachment #303769 - Flags: review? → review?(marco.zehe)
This is basically my test for all things that the idl file lists. The many todos prove my point from https://bugzilla.mozilla.org/show_bug.cgi?id=410052#c37 
Comment on attachment 303769 [details] [diff] [review]
nsIAccessibeTable mochitest

> Index: test_bug410052_1.html
...
> +   //columnHeaderIndex =columnHeader.getIndexAt(0,2);
> +   //is(columnHeaderIndex, 2, "columnheaderindex is wrong");

Did you rmean to remove these or add them back in at some point?

r=me with the above explained/fixed.
Attachment #303769 - Flags: review?(marco.zehe) → review+
files have wrong code style and I'd appretiate to use more readable file names than bug name because it's actually common tests. Bernd, Marco could you fix this, I guess we don't need approval to land this because it's tests only?
>files have wrong code style
I don't know how to fix this as this is very vague.
+    todo(works, "columnHeader should not throw");
+   //columnHeaderIndex =columnHeader.getIndexAt(0,2);
+   //is(columnHeaderIndex, 2, "columnheaderindex is wrong");

Currently the code throws, so the tests should be only added once the thing behaves as expected.
s/files have wrong code style/should follow the http://developer.mozilla.org/en/docs/JavaScript_style_guide/ ?
Comment on attachment 303769 [details] [diff] [review]
nsIAccessibeTable mochitest

I'll show some examples:

>+   var caption = accNode.caption;   
>+   is(caption.firstChild.name, "Test Table", "wrong text inside caption");
>+   const nsIAccess = Components.interfaces.nsIAccessible;

I would like to call it nsIAccessNode

>+   is(caption.children.queryElementAt(0, nsIAccess).name, "Test Table", "wrong text inside caption");

you shouldn't exceed 80 chars restriction.

>+   var range = document.createRange();
>+    range.selectNode(cell);

wrong indentation

>+   try
>+   {
>+     rowheader  = accNode.rowHeader;

iirc should be
try {
}

>+   }
>+   catch(e){

no whitespace between ( and }

>+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=410052">Mozilla Bug 410052</a>
>+  <p id="display"></p>

wrong inentation

>+  
>+</body></html>
>\ No newline at end of file

no new line :)

>+      var accNode = accService.getAccessibleFor(table).
>+      QueryInterface(Components.interfaces.nsIAccessibleTable);
>+
wron indentation
  


>+   if(s.rangeCount > 0) s.removeAllRanges();

should be 
if (s.rangeCount > 0)
  s.removeAllRanges();
Attached patch revised patchSplinter Review
I checked the testcases with the comments from Alexander in.
Closing INVALID, since these tests don't exist any more and we probably have coverage by now.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → INVALID
(In reply to Marco Zehe (:MarcoZ) from comment #21)
> Closing INVALID, since these tests don't exist any more and we probably have
> coverage by now.

these tests were renamed and actually this bug is fixed, changeset is http://hg.mozilla.org/mozilla-central/rev/9002bf7b7346
Resolution: INVALID → FIXED
You need to log in before you can comment on or make changes to this bug.