854868 - Make srcdir and objdir paths match for mochitest-a11y files

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[alexander :surkov (:asurkov)]

recently the way I used to run a11y mochitest stopped to work:
python runtests.py --a11y --test-path=accessible/text/test_multiline.html
gives me an error.

.mach doesn't allow me to run a single test:

mach --mochitest-a11y accessible/tests/mochitests/text/test_multiline.html
doesn't show any test (path relative top dir)

mach --mochitest-a11y accessible/text/test_multiline.html
gives me en error (path relative testing dir)

please fix this issue asap since it's a blocker for me

Comment 1

[David Bolter [:davidb] (NeedInfo me for attention)]

I ran into this yesterday as well :(

Comment 2

[(not currently active) Ted Mielczarek]

So, as a short-term workaround, you can replace the 'python' in your original command with '$objdir/_virtualenv/bin/python'. Alternately you can 'source $objdir/_virtualenv/bin/activate' to activate the in-tree virtualenv in this shell and then simply run 'python'.

Longer-term we should fix mach's mochitest-a11y to do what you need.

Comment 3

[Matt Brubeck (:mbrubeck)]

Attachment: patch

This patch just removes mach's srcdir file check, since the source path is not guaranteed to match the test root path.  We really need to move this check into the test harness, which knows where the test root is.

With this patch, you can use test-root-relative paths like "mach mochitest-a11y accessible/text/test_multiline.html"

Alternately, we could fix the relativesrcdir lines in the mochitest-a11y makefiles so that the srcdir and test-root paths were identical, e.g.:

http://hg.mozilla.org/mozilla-central/file/8d09f003e087/accessible/tests/mochitest/Makefile.in#l10

Then you could use the srcdir path and benefit from things like tab completion in the shell.

Comment 4

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

I would prefer landing <https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/c4b9a5aacf3f/a11y-mochitests>.

Comment 5

[Matt Brubeck (:mbrubeck)]

Attachment: alternate patch: make relativesrcdir = @relativesrcdir@

Comment 6

[(not currently active) Ted Mielczarek]

Yeah. Can we do that?

Comment 7

[Mike Hommey [:glandium]]

There are many other places where relativesrcdir is not @relativesrcdir@, can be change that too?

Comment 8

[Matt Brubeck (:mbrubeck)]

It looks like dbaron tried the relativesrcdir change in bug 847279 and ran into errors; any URIs that reference other /accessible/tests/mochitest files will need to be updated too.

Comment 9

[Gregory Szorc [:gps]]

I reckon bug 818819 may also help here.

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to Matt Brubeck (:mbrubeck) from comment #8)
> It looks like dbaron tried the relativesrcdir change in bug 847279 and ran
> into errors; any URIs that reference other /accessible/tests/mochitest files
> will need to be updated too.

I don't think these errors are that hard to fix; I just haven't gotten to it yet.  If somebody else wants to pick it up, that would be great.

Comment 11

[Trevor Saunders (:tbsaunde)]

> Then you could use the srcdir path and benefit from things like tab
> completion in the shell.

tab completion only works if cwd == topsrcdir which is suboptimal for lots of other things.

> I don't think these errors are that hard to fix; I just haven't gotten to it
> yet.  If somebody else wants to pick it up, that would be great.

I suspect you're right, but if we changing the path I'd prefer to get rid of the mochitest/ part at least to keep the path you need to type short.

Comment 12

[alexander :surkov (:asurkov)]

Comment on attachment 729562 [details] [diff] [review]
alternate patch: make relativesrcdir = @relativesrcdir@

it works, thank you

Comment 13

[alexander :surkov (:asurkov)]

(In reply to alexander :surkov from comment #12)
> Comment on attachment 729562 [details] [diff] [review]
> alternate patch: make relativesrcdir = @relativesrcdir@
> 
> it works, thank you

it has a problem, when running events/test_scroll.xul then it doesn't see longer the scroll.html file of the same folder (it says  /a11y/accessible/events/scroll.html was not found).

Comment 14

[Matt Brubeck (:mbrubeck)]

Attachment: relativesrcdir patch v2

Fixed some absolute paths.

I'm currently testing this locally, then I'll push it to Try.

Comment 15

[Gregory Szorc [:gps]]

mach now natively integrates with the Python mochitest runner as of bug 818819. That may have implications for this bug. (But not if we go down the relativesrcdir path.)

Comment 16

[Matt Brubeck (:mbrubeck)]

Attachment: patch v3

https://tbpl.mozilla.org/?tree=Try&rev=d79d527b1302

Comment 17

[Matt Brubeck (:mbrubeck)]

Attachment: relativesrcdir patch v4

Patch v3 is green on Try.  This is identical except for some whitespace fixes.

Comment 18

[Matt Brubeck (:mbrubeck)]

Attachment: move files from accessible/tests/mochitest to accessible/tests

At Trevor Saunders' request, this makes the test paths a bit shorter by removing the "mochitests" subdirectory.  Pushed to Try:

https://tbpl.mozilla.org/?tree=Try&rev=2796e9dfdd5c

Comment 19

[Trevor Saunders (:tbsaunde)]

Comment on attachment 730297 [details] [diff] [review]
relativesrcdir patch v4

stealing since I don't think surkov needs to spend time on this

Comment 20

[Trevor Saunders (:tbsaunde)]

Comment on attachment 730301 [details] [diff] [review]
move files from accessible/tests/mochitest to accessible/tests

looks correct, but lets be sure surkov thinks this is a good idea too since I suggested it :)

Comment 21

[alexander :surkov (:asurkov)]

Comment on attachment 730301 [details] [diff] [review]
move files from accessible/tests/mochitest to accessible/tests

it's good, I'm curious if add another kind of tests then where they will be located.

(btw, it'd be good to remove crashtests since we don't have any plans to ressurect them) (we have a bug iirc on this though)

Comment 22

[Matt Brubeck (:mbrubeck)]

By the way, bug 855563 (currently on inbound) fixes commands that use test root paths like "mach mochitest-a11y accessible/text/test_multiline.html"

So the patches in this bug are no longer required to make mach work, but they are still useful for other reasons.  Making srcdir and objdir paths match should reduce confusion, and make it easier to use tab completion to run test commands.

Comment 23

[alexander :surkov (:asurkov)]

I'm up for short paths sure.

Also I want to have one more test suite for accessibility at some point (https://wiki.mozilla.org/Accessibility/MATS). It'd be good to discuss ideas how we can organize folders when that day comes.

Comment 24

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #21)
> Comment on attachment 730301 [details] [diff] [review]
> move files from accessible/tests/mochitest to accessible/tests
> 
> it's good, I'm curious if add another kind of tests then where they will be
> located.

that bridge seems far enough out I'm tempted  to leave worrying about how  to cross it too another day

> (btw, it'd be good to remove crashtests since we don't have any plans to
> ressurect them) (we have a bug iirc on this though)

yeah, my only question is if we should rewrite the 1 existing one as a mochitest or if its obsolete.

Comment 25

[Trevor Saunders (:tbsaunde)]

Alex, do you plan to review that one patch or should I just cancel the request?

Comment 26

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #25)
> Alex, do you plan to review that one patch or should I just cancel the
> request?

I still not sure the patch is a good idea

(In reply to Trevor Saunders (:tbsaunde) from comment #24)
> (In reply to alexander :surkov from comment #21)
> > Comment on attachment 730301 [details] [diff] [review]
> > move files from accessible/tests/mochitest to accessible/tests
> > 
> > it's good, I'm curious if add another kind of tests then where they will be
> > located.
> 
> that bridge seems far enough out I'm tempted  to leave worrying about how 
> to cross it too another day

MATS project is one of our urgent things, I hope it's no so far so we can ignore it. In other words say we have a MATS tests today how we can put them into folders?

> > (btw, it'd be good to remove crashtests since we don't have any plans to
> > ressurect them) (we have a bug iirc on this though)
> 
> yeah, my only question is if we should rewrite the 1 existing one as a
> mochitest or if its obsolete.

we have XUL tree hit testing in mochitests already so remove it.

Comment 27

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #26)
> (In reply to Trevor Saunders (:tbsaunde) from comment #25)
> > Alex, do you plan to review that one patch or should I just cancel the
> > request?
> 
> I still not sure the patch is a good idea
> 
> (In reply to Trevor Saunders (:tbsaunde) from comment #24)
> > (In reply to alexander :surkov from comment #21)
> > > Comment on attachment 730301 [details] [diff] [review]
> > > move files from accessible/tests/mochitest to accessible/tests
> > > 
> > > it's good, I'm curious if add another kind of tests then where they will be
> > > located.
> > 
> > that bridge seems far enough out I'm tempted  to leave worrying about how 
> > to cross it too another day
> 
> MATS project is one of our urgent things, I hope it's no so far so we can
> ignore it. In other words say we have a MATS tests today how we can put them
> into folders?

well, nobody is working on it so I don't expect to see it immediately and changing things around for whatever we need is easy enough so I think we can more or less ignore it.  In the worst case we'd decide we'll decide in a couple months the best solution is to have accessible/tests/mochitest/ and accessible/tests/platform in which case doing this now made or lives better for a couple months at the cost of an hour of work.

I can  imagine any number of ways we could do things we could have accessible/src/mac/tests accessible/src/windows/tests/ etc or we could have just accessible/tests/ containing both mochitests and platform tests or we could have accessible/mochitests/ and accessible/platformtests/ and probably a number of other options among which I don't want to think about which I prefer until I know what the tests will look like.

> > > (btw, it'd be good to remove crashtests since we don't have any plans to
> > > ressurect them) (we have a bug iirc on this though)
> > 
> > yeah, my only question is if we should rewrite the 1 existing one as a
> > mochitest or if its obsolete.
> 
> we have XUL tree hit testing in mochitests already so remove it.

ok, I'll file a sepperate bug for that

Comment 28

[alexander :surkov (:asurkov)]

If we will end up with what we have then I wouldn't change anything now. 

Do we have anything here left except the latest patch?

Comment 29

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #28)
> If we will end up with what we have then I wouldn't change anything now. 

I'm hopeing we'll end up with something shorter, but who can say for sure

Comment 30

[Matt Brubeck (:mbrubeck)]

Should I at least land my @relativesrcdir@ patch for now?  That will mean typing slightly longer paths at the command line, but it will let you use tab completion to type the paths for you.  :)  Also it makes some of the tests more robust against breaking in case you do decide to move them in the future.

Comment 31

[Trevor Saunders (:tbsaunde)]

(In reply to Matt Brubeck (:mbrubeck) from comment #30)
> Should I at least land my @relativesrcdir@ patch for now?  That will mean
> typing slightly longer paths at the command line, but it will let you use
> tab completion to type the paths for you.  :)  Also it makes some of the
> tests more robust against breaking in case you do decide to move them in the
> future.

I don't see a reason why not, sorry to keep you waiting :(

Comment 32

[Matt Brubeck (:mbrubeck)]

Landing just the @relativesrcdir@ patch.  We can file a new bug if anyone wants to resurrect the second patch or move these tests to some other location.

https://hg.mozilla.org/integration/mozilla-inbound/rev/a7ec1554d481

Comment 33

[Trevor Saunders (:tbsaunde)]

(In reply to Matt Brubeck (:mbrubeck) from comment #32)
> Landing just the @relativesrcdir@ patch.  We can file a new bug if anyone
> wants to resurrect the second patch or move these tests to some other
> location.

well, I've wanted to take a patch like it for a long time, but never had the time to write it and now Alex doesn't seem to agree :(

Comment 34

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a7ec1554d481