Last Comment Bug 415730 - convert existing API tests into mochitests
: convert existing API tests into mochitests
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: Trunk
: x86 Windows XP
: -- normal (vote)
: ---
Assigned To: Marco Zehe (:MarcoZ)
:
Mentors:
Depends on: 416553
Blocks: a11ytestdev
  Show dependency treegraph
 
Reported: 2008-02-04 23:15 PST by Bernd
Modified: 2011-11-01 15:33 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
mochitest for the attributes of nsIAccessibleTable (3.67 KB, text/html)
2008-02-15 13:14 PST, Bernd
no flags Details
nsIAccessibeTable mochitest (12.89 KB, patch)
2008-02-16 13:03 PST, Bernd
mzehe: review+
Details | Diff | Splinter Review
revised patch (12.67 KB, patch)
2008-02-23 03:13 PST, Bernd
no flags Details | Diff | Splinter Review

Description Bernd 2008-02-04 23:15:15 PST
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.
Comment 2 Marco Zehe (:MarcoZ) 2008-02-05 01:01:06 PST
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.
Comment 3 alexander :surkov 2008-02-05 01:13:06 PST
Marco, please keep in mind to enable http://lxr.mozilla.org/mozilla/source/layout/xul/test/test_bug368835.xul test
Comment 4 alexander :surkov 2008-02-06 23:10:33 PST
remember about bug 410052 to add test for it
Comment 5 alexander :surkov 2008-02-07 01:20:10 PST
(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.
Comment 6 Rob Campbell [:rc] (:robcee) 2008-02-07 13:10:08 PST
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 alexander :surkov 2008-02-07 22:17:56 PST
(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.
Comment 8 Bernd 2008-02-15 13:14:43 PST
Created attachment 303599 [details]
mochitest for the attributes of nsIAccessibleTable
Comment 9 Bernd 2008-02-15 13:17:01 PST
the columnHeader is bug 417797
the rwoHeader    is bug 417791
Comment 10 Bernd 2008-02-15 23:59:07 PST
Just to avoid duplicates, I will write a complete set for nsIAccessibleTable.
Comment 11 Bernd 2008-02-16 13:03:57 PST
Created attachment 303769 [details] [diff] [review]
nsIAccessibeTable mochitest
Comment 12 Bernd 2008-02-16 13:08:42 PST
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 13 Marco Zehe (:MarcoZ) 2008-02-18 03:37:40 PST
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.
Comment 14 alexander :surkov 2008-02-18 06:44:29 PST
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?
Comment 15 Bernd 2008-02-18 12:09:59 PST
>files have wrong code style
I don't know how to fix this as this is very vague.
Comment 16 Bernd 2008-02-18 12:12:23 PST
+    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.
Comment 17 Bernd 2008-02-18 22:07:41 PST
s/files have wrong code style/should follow the http://developer.mozilla.org/en/docs/JavaScript_style_guide/ ?
Comment 18 alexander :surkov 2008-02-18 22:17:27 PST
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();
Comment 19 Bernd 2008-02-23 03:13:33 PST
Created attachment 305155 [details] [diff] [review]
revised patch
Comment 20 Bernd 2008-02-23 03:39:15 PST
I checked the testcases with the comments from Alexander in.
Comment 21 Marco Zehe (:MarcoZ) 2011-11-01 08:03:19 PDT
Closing INVALID, since these tests don't exist any more and we probably have coverage by now.
Comment 22 alexander :surkov 2011-11-01 15:33:02 PDT
(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

Note You need to log in before you can comment on or make changes to this bug.