Closed Bug 1337887 Opened 8 years ago Closed 7 years ago

Get mochitest a11y tests running reliably with e10s turned on (or convert them to bc tests)

Categories

(Core :: Disability Access APIs, defect, P2)

All
Windows
defect

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: jimm, Assigned: eeejay)

References

Details

(Whiteboard: aes-)

Attachments

(1 file)

For a11y coverage, we want to get these running. These tests cover core a11y functionality. We currently have them running under non-e10s which eventually we'll want to turn off. * Windows 7 opt: non-e10s * Windows 7 pgo: no coverage * Windows 7 VM debug: no coverage * Windows 8 opt: no coverage * Windows 8 x64 opt: non-e10s * Windows 8 x64 pgo: non-e10s * Windows 8 x64 debug: non-e10s * Windows 10 x64 VM opt: no coverage * Windows 10 x64 VM debug: no coverage * Windows 2012 opt: no coverage * Windows 2012 pgo: no coverage * Windows 2012 debug: no coverage * Windows 2012 x64 opt: no coverage * Windows 2012 x64 pgo: no coverage * Windows 2012 x64 debug: no coverage Also need to take a look at linux and osx to see where we are there.
So we're on the same page, mochitest-other consists these days of only mochitest-a11y and mochitest-chrome. For mochitest-chrome, we did a thorough audit back during the initial e10s rollout and ported many tests over to other suites (with remaining tests being deemed not-applicable to e10s). So I don't think there's anything to do there still unless we want to take another look at the remaining tests to ensure that nothing else needs porting? For mochitest-a11y, it's my understanding that Yura did a similar audit and porting effort. But NI him to confirm. Assuming the above is all accurate, I'm not sure there's anything that needs to be done here?
Flags: needinfo?(yzenevich)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #1) > So we're on the same page, mochitest-other consists these days of only > mochitest-a11y and mochitest-chrome. > > For mochitest-chrome, we did a thorough audit back during the initial e10s > rollout and ported many tests over to other suites (with remaining tests > being deemed not-applicable to e10s). So I don't think there's anything to > do there still unless we want to take another look at the remaining tests to > ensure that nothing else needs porting? > > For mochitest-a11y, it's my understanding that Yura did a similar audit and > porting effort. But NI him to confirm. > > Assuming the above is all accurate, I'm not sure there's anything that needs > to be done here? Within a year or so, we'll stop supporting the non-e10s run mode, so I don' think we want to continue to run any test suite under it. Once we get these switched over to e10s, we can turn non-e10s runs off.
(In reply to Ryan VanderMeulen [:RyanVM] from comment #1) > So we're on the same page, mochitest-other consists these days of only > mochitest-a11y and mochitest-chrome. > > For mochitest-chrome, we did a thorough audit back during the initial e10s > rollout and ported many tests over to other suites (with remaining tests > being deemed not-applicable to e10s). So I don't think there's anything to > do there still unless we want to take another look at the remaining tests to > ensure that nothing else needs porting? > > For mochitest-a11y, it's my understanding that Yura did a similar audit and > porting effort. But NI him to confirm. > > Assuming the above is all accurate, I'm not sure there's anything that needs > to be done here? So I ported a number of tests that are relevant to e10s, but not all. I'm not sure if these tests fail with e10s enabled, since they run in the same process. If they pass we should just run them, if there are issues, ideally, they should all be ported to bc type, and definitely new ones should all be bc.
Flags: needinfo?(yzenevich)
Also wondering what David's and Alex's thoughts are about this..
Flags: needinfo?(surkov.alexander)
Flags: needinfo?(dbolter)
Here's the current situation as I understand it: Yura ported some mochitests to ensure we are testing IPC as well, these are: https://dxr.mozilla.org/mozilla-central/source/accessible/tests/browser This is good and gives us decent confidence for correctness including IPC. There are a lot more remaining correctness mochitests: https://dxr.mozilla.org/mozilla-central/source/accessible/tests/mochitest These run in single process mode in 'oth'. We want to know if the can be run in multi-process even though they wouldn't test IPC (presumably hoping that the test suite can run in the content process and still show up as green/passing on tbpl)? I don't recall this working but would be good to confirm.
Flags: needinfo?(dbolter)
Depends on: 1338610
Assignee: jmathies → nobody
Assignee: nobody → yzenevich
Got a Try run of mochitest-a11y with e10s forced on: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a82d9f6dd2333778959595b6b61843149db0a0f6&group_state=expanded It's a start anyway! :-) Also, here's the patch to use for subsequent pushes to Try: https://hg.mozilla.org/try/raw-rev/3f267f8f28dbf36c6a3289f044edac83298f4e87
I think the confusion with e10s/disable-e10s logging comes from here: https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/TestRunner.js#465 With the patch we default to e10s however the TestRunner will report e10s only if current process type is not default which, I guess, is the same as parent.
(In reply to Ryan VanderMeulen [:RyanVM] from comment #6) > Got a Try run of mochitest-a11y with e10s forced on: > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=a82d9f6dd2333778959595b6b61843149db0a0f6&group_state=e > xpanded > > It's a start anyway! :-) > > Also, here's the patch to use for subsequent pushes to Try: > https://hg.mozilla.org/try/raw-rev/3f267f8f28dbf36c6a3289f044edac83298f4e87 I think for local testing, e.g. changes in testing/mochitest/runtests.py we need to still keep chrome check and only remove a11y?
I don't think it'd really matter one way or the other.
Yura what's next here?
Flags: needinfo?(yzenevich)
Ideally I would want to see all our tests written as b-c tests: some of the tests that deal with content across different frames will are failing and writing the mochitest that will pass will take as long as writing a b-c test. Other tests should work in theory with e10s enabled but some fail for reasons that need further investigation.
Flags: needinfo?(yzenevich)
Flags: needinfo?(surkov.alexander)
Not blocking rollout. We can continue to run the existing a11y mochitest suite under non-e10s for now. The tests cover what they need to test under the current configuration.
Summary: Get OTH tests running under e10s on Windows → Get mochitest a11y tests running reliably with e10s turned on (or convert them to bc tests)
Whiteboard: aes+ → aes-
Component: General → Disability Access APIs
Assignee: yzenevich → eitan
Pulled out all the problematic tests (and tried to fix one). This try will be maybe green-ish. https://treeherder.mozilla.org/#/jobs?repo=try&revision=99d992ed5e111f082cf77db15c3d7b32387ec73b The rest of the tests will either need to be fixed or ported to browser tests.
Depends on: 1367486
Depends on: 1368182
Depends on: 1369890
Depends on: 1372285
Depends on: 1376852
Depends on: 1376857
I went back to see if there is a simpler way of fixing our current mochitests, it seems like most of the issues are opening a remote window and failing to get a reference to its DOM. If we force it to be non-remote, most of these tests should work in "e10s" mode as well. The orginal force-e10s patch RyanVM put here stopped working, so I am not 100% convinced e10s is force on in all cases. But here is a try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6756b84c66bfd9a62b3acf75bc8c229fcc1ad863 The two tests that fail in e10s are the plugin and <embed> test in Windows. RyanVM, does this look promising?
Flags: needinfo?(ryanvm)
Per our IRC discussion, this looks good I think. But we should verify that this approach won't break if/when support for non-remote windows breaks.
Flags: needinfo?(ryanvm)
Sorry for dropping this. This try push is the best ever. https://treeherder.mozilla.org/#/jobs?repo=try&revision=8cd94495033ab15d9f0e3eeb230032884d7cd328 Blake, can we rely on opening windows in non-remote mode for testing purposes?
Flags: needinfo?(mrbkap)
I don't know how much I'd rely on that. I get the impression that folks are pretty eager to stop supporting e10s once a11y supports it and I'd imagine that would include cutting support for loading foreign content in the parent process. Gabor or Felipe might have more thoughts about this, though.
Flags: needinfo?(mrbkap)
Hey Eitan, what's next here?
Flags: needinfo?(eitan)
Priority: -- → P2
I think opening the windows non-remote is a no-brainer for now. So we should just push that. After that, we will have a couple of tests to tackle. If non-remote won't be an option in the future, we'll figure it out.
Flags: needinfo?(eitan)
Comment on attachment 8906144 [details] [diff] [review] Open new browser windows as non-remote in a11y mochitests. r?yzen Review of attachment 8906144 [details] [diff] [review]: ----------------------------------------------------------------- Looks good,thanks
Attachment #8906144 - Flags: review?(yzenevich) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/cfdf38e0d9be Open new browser windows as non-remote in a11y mochitests. r=yzen
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: