convert existing API tests into mochitests

RESOLVED FIXED

Status

()

Core
Disability Access APIs
RESOLVED FIXED
10 years ago
6 years ago

People

(Reporter: Bernd, Assigned: MarcoZ)

Tracking

(Blocks: 1 bug)

Trunk
x86
Windows XP
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

10 years ago
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.
(Reporter)

Comment 1

10 years ago
this are the really good ones
http://www.mozilla.org/projects/ui/accessibility/unix/testcase/nsIAccExt/TestCases_for_HTML_Elements.html
(Assignee)

Comment 2

10 years ago
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)

Updated

10 years ago
Assignee: aaronleventhal → marco.zehe
Status: ASSIGNED → NEW
(Assignee)

Updated

10 years ago
Status: NEW → ASSIGNED

Comment 3

10 years ago
Marco, please keep in mind to enable http://lxr.mozilla.org/mozilla/source/layout/xul/test/test_bug368835.xul test

Comment 4

10 years ago
remember about bug 410052 to add test for it

Comment 5

10 years ago
(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.

Comment 7

10 years ago
(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.

Updated

10 years ago
Depends on: 416553
(Assignee)

Updated

10 years ago
Blocks: 417472
(Reporter)

Comment 8

10 years ago
Created attachment 303599 [details]
mochitest for the attributes of nsIAccessibleTable
(Reporter)

Comment 9

10 years ago
the columnHeader is bug 417797
the rwoHeader    is bug 417791
(Reporter)

Comment 10

10 years ago
Just to avoid duplicates, I will write a complete set for nsIAccessibleTable.
(Reporter)

Comment 11

10 years ago
Created attachment 303769 [details] [diff] [review]
nsIAccessibeTable mochitest
Attachment #303769 - Flags: review?
(Reporter)

Updated

10 years ago
Attachment #303769 - Flags: review? → review?(marco.zehe)
(Reporter)

Comment 12

10 years ago
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 
(Assignee)

Comment 13

10 years ago
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+

Comment 14

10 years ago
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?
(Reporter)

Comment 15

10 years ago
>files have wrong code style
I don't know how to fix this as this is very vague.
(Reporter)

Comment 16

10 years ago
+    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.
(Reporter)

Comment 17

10 years ago
s/files have wrong code style/should follow the http://developer.mozilla.org/en/docs/JavaScript_style_guide/ ?

Comment 18

10 years ago
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();
(Reporter)

Comment 19

10 years ago
Created attachment 305155 [details] [diff] [review]
revised patch
(Reporter)

Comment 20

10 years ago
I checked the testcases with the comments from Alexander in.
(Assignee)

Comment 21

6 years ago
Closing INVALID, since these tests don't exist any more and we probably have coverage by now.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → INVALID

Comment 22

6 years ago
(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.