416553 - option to run accessibility tests

Status: VERIFIED FIXED

(Testing :: Mochitest, defect)

Description

[alexander :surkov (:asurkov)]

spun off bug 415730

Rob wrote:

(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.

attached file works though it doesn't allow to run all tests. I cannot find where similar feature of usual tests are implemented.

Comment 1

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 2

[Bernd]

will the a11y tests run with chrome privilegs?

Comment 3

[alexander :surkov (:asurkov)]

(In reply to comment #2)
> will the a11y tests run with chrome privilegs?
> 

they need to operate with XPCOM.

Comment 4

[alexander :surkov (:asurkov)]

Attachment: patch2

Comment 5

[Jeff Walden [:Waldo]]

Why can't you use the existing --chrome method of running tests for this?  Under that setup, tests run from chrome://mochitest/content/<something more I can't remember>, and you have chrome privileges, can open XUL windows to avoid influence of browser chrome, etc.  From my point of view, the fewer functionality-specific tests pushed into runtests.p[ly].in, the better, and I don't see anything at a glance that requires this to be pushed into a type of testing.

Comment 6

[alexander :surkov (:asurkov)]

Attachment: patch3

use -chrome method as Jeff suggested

Comment 7

[Jeff Walden [:Waldo]]

That's not quite what I suggested -- what I meant was don't even change runtests.p[yl].in or any of the test harness at all (so nothing in testing/mochitest would change), and just run the accessibility tests within the --chrome setup.  The tests get run from chrome: URLs, and I don't see what you can't do with them that you can only do with yet another flavor of Mochitest harness.

Comment 8

[alexander :surkov (:asurkov)]

Yes, but that's Rob suggested to have an option to run accessibility tests separately at least because some mochitests become broken with enabled accessibility. Sure those tests should fail with enabled accessibility but I think it's ok to have an ability to run accessibility related tests separately.

Comment 9

[alexander :surkov (:asurkov)]

Attachment: patch4

added testcase for bug 308564.

Comment 10

[Rob Campbell [:rc] (:robcee)]

(In reply to comment #7)
> That's not quite what I suggested -- what I meant was don't even change
> runtests.p[yl].in or any of the test harness at all (so nothing in
> testing/mochitest would change), and just run the accessibility tests within
> the --chrome setup.  The tests get run from chrome: URLs, and I don't see what
> you can't do with them that you can only do with yet another flavor of
> Mochitest harness.

Jeff: I'd prefer having the a11y tests running separately at the moment. It's probably possible to use the --chrome option with some specific prefs enabled to run the tests, but I didn't want to contaminate any of the existing test harnesses with side-effects (or the main tree).

Thanks for the patches, alexander. I'll get to a review in a couple of days when I've dug out of my bugmail.

Comment 11

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 302599 [details] [diff] [review]
patch4

one pervasive nit: change occurrences of "ally" to "a11y" (l->1) and "ALLY" to "A11Y". This includes filenames, directories, code and comments.

remove hard tabs in mozilla/testing/mochitest/harness-overlay.xul.

otherwise, I think this patch looks good. I didn't have an a11y tests to run in it, but the harness brings up minefield (on mac, at least). I also checked that it didn't break --chrome or --browser-chrome and those seemed fine as well.

I'm going to give a conditional r+ based on the above nits being addressed.

Thanks for your help!

Comment 12

[alexander :surkov (:asurkov)]

Rob, I forgot to fix your comments and checked in the patch as is. Your comments doesn't look critical so let me do this in following up patch.

Comment 13

[alexander :surkov (:asurkov)]

(In reply to comment #11)
> (From update of attachment 302599 [details] [diff] [review])
> one pervasive nit: change occurrences of "ally" to "a11y" (l->1) and "ALLY" to
> "A11Y". This includes filenames, directories, code and comments.

though not for this one. Rob if you really don't like ally (instead of a11y) though they both are used in the same sense then for sure I'll back out the patch and will fix it.

Comment 14

[alexander :surkov (:asurkov)]

Ok. I looked at usage of ally and a11y. It sounds ally word is used not offical, in the speach. So I'm going to back up the patch to fix ally/a11y.

Comment 15

[alexander :surkov (:asurkov)]

checked in with Rob's comments.

Comment 16

[Rob Campbell [:rc] (:robcee)]

alexander: in the future, if someone gives you a conditional review, make the changes before checking in, especially if the nits involve file and directory naming. It's a pain to rename patches after the fact.

Also, when checking in, please, include the cvs logs in the bug either as an attachment or appended as a comment. Thanks.

Comment 17

[alexander :surkov (:asurkov)]

(In reply to comment #16)
> alexander: in the future, if someone gives you a conditional review, make the
> changes before checking in, especially if the nits involve file and directory
> naming. It's a pain to rename patches after the fact.

Sure, I just missed this, sorry. But in the end I checked in patch with your comments.

> Also, when checking in, please, include the cvs logs in the bug either as an
> attachment or appended as a comment. Thanks.
> 

If it should be helpful then I will.

Comment 18

[Marco Zehe (:MarcoZ)]

Alexander, currently, this --a11y switch only picks up .xul files/chrome tests, right? However, we need to be able to also run HTML-based tests. These are equally important. Right now, no HTML files are picked up at all if I run with the --a11y switch.

Comment 19

[alexander :surkov (:asurkov)]

(In reply to comment #18)
> Alexander, currently, this --a11y switch only picks up .xul files/chrome tests,
> right?

Not sure I got you. -a11y picks up tests from _tests/testing/mochitests/a11y directory. Currently only accessible/tests/mochitest tests are moved there.

 However, we need to be able to also run HTML-based tests. These are
> equally important. Right now, no HTML files are picked up at all if I run with
> the --a11y switch.
> 

I tried html mochitest from bug 416742 it works fine.

Comment 20

[Marco Zehe (:MarcoZ)]

(In reply to comment #19)
> I tried html mochitest from bug 416742 it works fine.

What command did you use to run it? Did it show up when you simply ran runtests.py --a11y? It doesn't for me.

Comment 21

[Rob Campbell [:rc] (:robcee)]

(In reply to comment #17)
> (In reply to comment #16)
> > alexander: in the future, if someone gives you a conditional review, make the
> > changes before checking in, especially if the nits involve file and directory
> > naming. It's a pain to rename patches after the fact.
> 
> Sure, I just missed this, sorry. But in the end I checked in patch with your
> comments.

Yes, true. I'm just saying that when dealing with the source repository order does matter. I think had you tried to do the changes in place with the code checked in already, it would have required backing out and then re-submitting. A seemingly minor point that a correct order would have avoided.

Anyway, you got it and that's the main thing. :)(In reply to comment #17)
> > Also, when checking in, please, include the cvs logs in the bug either as an
> > attachment or appended as a comment. Thanks.
> 
> If it should be helpful then I will.

It's useful because you can see the version numbers associated with the files in the bug itself if there's ever a need to backout a change.

Comment 22

[alexander :surkov (:asurkov)]

(In reply to comment #20)
> (In reply to comment #19)
> > I tried html mochitest from bug 416742 it works fine.
> 
> What command did you use to run it? Did it show up when you simply ran
> runtests.py --a11y? It doesn't for me.
> 

Did you put it into accessible/tests/mochitest, change makefile and build the tree? Or did you put it directly to _tests/testing/mochitests/a11y/accessible? I'll try one more time today.

Comment 23

[alexander :surkov (:asurkov)]

Marco, if you put it into the right directory then you probably gave it wrong name, the file name should contain "test" word to be recognized as mochitest. For that mochitest I use "test_bug410052.html". It works fine with me.

Comment 24

[Marco Zehe (:MarcoZ)]

(In reply to comment #23)
> Marco, if you put it into the right directory then you probably gave it wrong
> name, the file name should contain "test" word to be recognized as mochitest.
> For that mochitest I use "test_bug410052.html". It works fine with me.
That was it, works now, thanks!