Closed
Bug 847279
Opened 8 years ago
Closed 8 years ago
some makefiles have bad relativesrcdir, making it hard to find tests
Categories
(Core :: General, defect)
Core
General
Tracking
()
RESOLVED
FIXED
mozilla23
People
(Reporter: dbaron, Assigned: bkelly)
References
Details
Attachments
(4 files, 6 obsolete files)
7.62 KB,
patch
|
mbrubeck
:
review+
khuey
:
feedback+
mossop
:
feedback+
briansmith
:
feedback+
RyanVM
:
checkin+
|
Details | Diff | Splinter Review |
52.22 KB,
patch
|
mbrubeck
:
review+
khuey
:
feedback+
mossop
:
feedback+
dbaron
:
feedback+
briansmith
:
feedback+
RyanVM
:
checkin+
|
Details | Diff | Splinter Review |
967 bytes,
patch
|
Dolske
:
review+
mbrubeck
:
checkin+
|
Details | Diff | Splinter Review |
2.28 KB,
patch
|
mbrubeck
:
review+
dholbert
:
feedback+
RyanVM
:
checkin+
|
Details | Diff | Splinter Review |
There are some makefiles where relativesrcdir is not the same as the location of the makefile. Since relativesrcdir is used for exporting tests, this ends up making it hard to find tests from error messages (because the path doesn't match), and hard to run tests after modifying them (because the TEST_PATH argument is different from what is expected). I tried to mass-fix these at one point in the past, but ran into the problem that some tests depended on the incorrect relativesrcdir. We should fix these.
Reporter | ||
Comment 1•8 years ago
|
||
Let's see what happens: https://tbpl.mozilla.org/?tree=Try&rev=ecfa9a8ebf09 Those 2 patches don't quite fix all of them. But I started from: find * -name Makefile.in -exec sed -i -e 's/^\(relativesrcdir[ \t]*=[ \t]*\).*/\1@relativesrcdir@/' {} \; and then pared down the changes.
Reporter | ||
Comment 2•8 years ago
|
||
The patches I'm testing are: https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/c4b9a5aacf3f/a11y-mochitests https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/c4b9a5aacf3f/fix-relativesrcdir and the changes I dropped were: https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-rev/c4b9a5aacf3f
Comment 3•8 years ago
|
||
You should be able to just remove the relativesrcdir from dom/imptests/Makefile.in.
Reporter | ||
Comment 4•8 years ago
|
||
(In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from comment #1) > Let's see what happens: > https://tbpl.mozilla.org/?tree=Try&rev=ecfa9a8ebf09 > > Those 2 patches don't quite fix all of them. But I started from: > find * -name Makefile.in -exec sed -i -e 's/^\(relativesrcdir[ \t]*=[ > \t]*\).*/\1@relativesrcdir@/' {} \; > and then pared down the changes. The problems were: mochitest-3 and mochitest-ipcplugins: 08:40:35 INFO - 10746 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/plugins/test/mochitest/test_redirect_handling.html | Test for expected stream notification status. - got 1, expected 0 mochitest-5: 00:01:59 INFO - 163651 ERROR TEST-UNEXPECTED-FAIL | /tests/security/manager/ssl/tests/mochitest/mixedcontent/test_bug329869.html | Test timed out. 00:07:29 INFO - 163656 ERROR TEST-UNEXPECTED-FAIL | /tests/security/manager/ssl/tests/mochitest/mixedcontent/test_bug383369.html | Test timed out. 00:12:59 INFO - 163663 ERROR TEST-UNEXPECTED-FAIL | /tests/security/manager/ssl/tests/mochitest/mixedcontent/test_bug455367.html | Test timed out. 00:18:29 INFO - 163668 ERROR TEST-UNEXPECTED-FAIL | /tests/security/manager/ssl/tests/mochitest/mixedcontent/test_bug472986.html | Test timed out. 00:18:29 INFO - 163669 ERROR TEST-UNEXPECTED-FAIL | (SimpleTest/TestRunner.js) | 4 test timeouts, giving up. 00:18:29 INFO - 163670 ERROR TEST-UNEXPECTED-FAIL | (SimpleTest/TestRunner.js) | Skipping 105 remaining tests. 00:18:30 INFO - 163671 ERROR TEST-UNEXPECTED-FAIL | (SimpleTest/TestRunner.js) | /tests/security/manager/ssl/tests/mochitest/mixedcontent/test_bug472986.html finished in a non-clean fashion, probably because it didn't call SimpleTest.finish() mochitest-chrome: 23:52:51 INFO - 4222 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/dom/plugins/test/mochitest/test_privatemode_perwindowpb.xul | uncaught exception - NS_ERROR_FILE_NOT_FOUND: Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIWebNavigation.loadURI] at chrome://global/content/bindings/browser.xml:156 23:52:52 INFO - 4224 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/dom/plugins/test/mochitest/test_privatemode_perwindowpb.xul | Exception thrown getting private mode state. - got true, expected false 23:52:52 INFO - 4225 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/dom/plugins/test/mochitest/test_privatemode_perwindowpb.xul | Querying private mode reported incorrectly - got undefined, expected true 23:52:52 INFO - 4226 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/dom/plugins/test/mochitest/test_privatemode_perwindowpb.xul | Querying private mode reported incorrectly - got undefined, expected true 23:52:52 INFO - 4227 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/dom/plugins/test/mochitest/test_privatemode_perwindowpb.xul | uncaught exception - TypeError: pluginElement1 is null at chrome://mochitests/content/chrome/dom/plugins/test/mochitest/test_privatemode_perwindowpb.xul:91 23:52:52 INFO - 4228 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/dom/plugins/test/mochitest/test_privatemode_perwindowpb.xul | [SimpleTest.finish()] this test already called finish! 23:52:52 INFO - 4229 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/dom/plugins/test/mochitest/test_privatemode_perwindowpb.xul | called finish() multiple times 23:58:12 INFO - 4245 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/dom/plugins/test/mochitest/test_xulbrowser_plugin_visibility.xul | Test timed out. 23:58:52 INFO - 9951 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/dom/tests/mochitest/localstorage/test_localStorageBasePrivateBrowsing_perwindowpb.html | private browsing values threw away - got private browsing value, expected null 23:58:52 INFO - 9952 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/dom/tests/mochitest/localstorage/test_localStorageBasePrivateBrowsing_perwindowpb.html | No items - got 1, expected 0 00:00:46 INFO - 12041 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/layout/base/tests/chrome/test_bug551434.html | uncaught exception - TypeError: frames[0].document.getElementById(...) is null at chrome://mochitests/content/chrome/layout/base/tests/chrome/test_bug551434.html:1 00:00:46 INFO - 12045 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/layout/base/tests/chrome/test_bug551434.html | keydown on input 4 - got undefined, expected 0 00:00:46 INFO - 12046 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/layout/base/tests/chrome/test_bug551434.html | keypress on input 4 - got undefined, expected 0 00:00:46 INFO - 12047 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/layout/base/tests/chrome/test_bug551434.html | keyup on input 4 - got undefined, expected 0 00:00:46 INFO - 12049 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/layout/base/tests/chrome/test_bug551434.html | uncaught exception - TypeError: i4 is null at chrome://mochitests/content/chrome/layout/base/tests/chrome/test_bug551434.html:68 00:00:46 INFO - 12050 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/layout/base/tests/chrome/test_bug551434.html | [SimpleTest.finish()] this test already called finish! 00:00:46 INFO - 12051 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/layout/base/tests/chrome/test_bug551434.html | called finish() multiple times 00:04:26 INFO - 14034 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/security/manager/ssl/tests/mochitest/stricttransportsecurity/test_sts_privatebrowsing_perwindowpb.html | in ROUND plain, test samedom - got INSECURE, expected SECURE 00:04:28 INFO - 14048 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/security/manager/ssl/tests/mochitest/stricttransportsecurity/test_sts_privatebrowsing_perwindowpb.html | in ROUND subdom, test samedom - got INSECURE, expected SECURE 00:04:28 INFO - 14050 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/security/manager/ssl/tests/mochitest/stricttransportsecurity/test_sts_privatebrowsing_perwindowpb.html | in ROUND subdom, test subdom - got INSECURE, expected SECURE 00:05:38 INFO - 16118 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/components/places/tests/test_bug_461710_perwindowpb.html | uncaught exception - TypeError: gIframe is null at chrome://mochitests/content/chrome/toolkit/components/places/tests/test_bug_461710_perwindowpb.html:106 00:15:55 INFO - 32137 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/mozapps/downloads/tests/chrome/test_close_on_last_window.xul | Test timed out. 00:15:59 INFO - 32142 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/mozapps/downloads/tests/chrome/test_delete_key_removes.xul | [SimpleTest.finish()] this test already called finish! 00:16:39 INFO - 32213 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/mozapps/downloads/tests/chrome/test_space_key_pauses_resumes.xul | file was downloaded - got http://example.com/httpd.js, expected http://mochi.test:8888/chrome/toolkit/mozapps/downloads/tests/chrome/unknownContentType_dialog_layout_data.pif 00:16:40 INFO - 32215 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/mozapps/downloads/tests/chrome/test_space_key_pauses_resumes.xul | file destination was set 00:16:40 INFO - 32216 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/mozapps/downloads/tests/chrome/test_space_key_pauses_resumes.xul | file name was set 00:16:40 INFO - 32217 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/mozapps/downloads/tests/chrome/test_space_key_pauses_resumes.xul | called finish() multiple times 00:19:36 INFO - 36997 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/widget/tests/test_bug462106_perwindow.xul | Data no longer available after leaving the private browsing mode - didn't expect another random number: 0.9816515821892492, but got it mochitest-a11y: 00:27:44 INFO - 4462 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/tests/mochitest/hittest/test_zoom_tree.xul | Test timed out. 00:27:46 INFO - 4463 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/tests/mochitest/hittest/test_zoom_tree.xul | [SimpleTest.finish()] waitForFocus() was called a different number of times from the number of callbacks run. Maybe the test terminated prematurely -- be sure to use SimpleTest.waitForExplicitFinish(). - got 1, expected 0 00:33:15 INFO - 5471 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/tests/mochitest/pivot/test_virtualcursor.html | Test timed out. 00:33:16 INFO - 5472 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/tests/mochitest/pivot/test_virtualcursor.html | [SimpleTest.finish()] waitForFocus() was called a different number of times from the number of callbacks run. Maybe the test terminated prematurely -- be sure to use SimpleTest.waitForExplicitFinish(). - got 1, expected 0 00:39:29 WARNING - TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/tests/mochitest/states/test_doc_busy.html | application timed out after 330 seconds with no output 00:39:36 INFO - PROCESS-CRASH | chrome://mochitests/content/a11y/accessible/tests/mochitest/states/test_doc_busy.html | application crashed [@ libc-2.11.so + 0xd4aa3] 00:39:40 ERROR - Return code: 1
Reporter | ||
Comment 5•8 years ago
|
||
Current versions of the patches are: https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/tip/a11y-mochitests https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/tip/fix-relativesrcdir
Assignee | ||
Comment 6•8 years ago
|
||
Fixes for the mochitest-chrome failures. Builds off of patches referenced in comment 5. Note, I could note reproduce errors for the following tests. They look like some may be intermittent timing issues. - dom/plugins/test/mochitest/test_xulbrowser_plugin_visibility.xul - toolkit/mozapps/downloads/tests/chrome/test_close_on_last_window.xul - toolkit/mozapps/downloads/tests/chrome/test_delete_key_removes.xul - toolkit/mozapps/downloads/tests/chrome/test_space_key_pauses_resumes.xul
Assignee | ||
Comment 7•8 years ago
|
||
Updated patch with fixes for: * dom/plugins/test/mochitest/test_redirect_handling.html * security/manager/ssl/tests/mochitest/mixedcontent/* Note, some of the mixedcontent tests were strangely sensitive to end-of-file characters. That seemed like a bug to me, but I did not have time to investigate if it was just a problem with the mochitest or the code under test. Just need to fix the ally tests and then this initial batch of errors should be done.
Attachment #732191 -
Attachment is obsolete: true
Assignee | ||
Comment 8•8 years ago
|
||
Actually, ignore that bit about sensitivity to end-of-file newlines. It appears its just the browser accepting content outside of the <body> element as if it was in the body for compatibility. This causes the end-of-file newline to show up in the body which throws off the innerHTML equality to the empty string in some tests.
Assignee | ||
Comment 9•8 years ago
|
||
Updated with a11y fixes and a few more errors I found. My poor 4-yo laptop is taking its time running the full suite and my try-server account is not active yet. If anyone could run this through try, that would be great.
Attachment #733717 -
Attachment is obsolete: true
Comment 10•8 years ago
|
||
Here you go: windows, mac, and linux, all mochitest suites. https://tbpl.mozilla.org/?tree=Try&rev=587fe00f45b5
Assignee | ||
Comment 11•8 years ago
|
||
Thanks! Did you include dbaron's patches from comment 5? I wasn't sure if I should roll those in or add them as separate attachments, etc. I'm new here.
Assignee | ||
Comment 12•8 years ago
|
||
Assignee | ||
Comment 13•8 years ago
|
||
Assignee | ||
Comment 14•8 years ago
|
||
Yea, looks like the build failed because I hadn't added dbaron's patches. Sorry about that.
Comment 15•8 years ago
|
||
Thanks for clearing that up! https://tbpl.mozilla.org/?tree=Try&rev=a2a148a876de
Assignee | ||
Comment 16•8 years ago
|
||
Thanks, that build went much better. Looks like there are still errors in: * accessible/jsat/test_utterance_order.html * toolkit/components/satchel/test/test_bug_511615.html * toolkit/components/satchel/test/test_form_autocomplete.html It looks like test_utterance_order.html was just added and its relativesrcdir will need to be fixed as well. The two satchel tests pass on my macbook. From the try output, it looks like they only fail on win7/8. Seems unlikely to be related to the relativesrcdir changes, but I haven't looked yet.
Assignee | ||
Comment 17•8 years ago
|
||
Also these two appear related to the relativesrcdir changes, but are windows specific tests: * dom/plugins/test/mochitest/test_busy_hang.xul * dom/plugins/test/mochitest/test_idle_hang.xul
Assignee | ||
Comment 18•8 years ago
|
||
Updated to fix the errors in: * accessible/jsat/test_utterance_order.html * dom/plugins/test/mochitest/test_busy_hang.xul * dom/plugins/test/mochitest/test_idle_hang.xul The two dom tests are untested since I don't have a windows dev environment currently. The changes looked straightforward, though.
Attachment #734249 -
Attachment is obsolete: true
Comment 19•8 years ago
|
||
Shout if you'd like another push to try.
Assignee | ||
Comment 20•8 years ago
|
||
So I don't think the errors in toolkit/components/satchel/test are related to the relativesrcdir changes. The relativesrcdir was not modified in this directory. Josh, would you be so kind as to kick off another try? Thanks! If this resolves the errors, are you able to review/commit? Or should I flag dbaron for when he returns from vacation?
Duplicate of this bug: 859438
Updated•8 years ago
|
Comment 22•8 years ago
|
||
Comment on attachment 734260 [details] [diff] [review] dbaron's a11y patch from comment 5. I just landed a similar change in bug 847279 -- sorry for the duplicate work.
Attachment #734260 -
Attachment is obsolete: true
Assignee | ||
Comment 23•8 years ago
|
||
No problem. I think attachment 734406 [details] [diff] [review] will conflict in a couple places with you patch. I can fix that later this evening if no one else gets to it before then. Also, it looks like you have a better solution using getRootDirectory(). I wasn't aware of that call. I'll look to see if any of my other changes would be better fixed using that. Thanks!
Assignee | ||
Comment 24•8 years ago
|
||
Updated to account for fixes from bug 854868. Also attempt to use getRootDirectory() where possible to avoid hardcoding paths. I only found one place that could do this, though. (Seems to only work for chrome:// URLs?) Still waiting for my level 1 access, so if jdm or someone else could kick off a try that would be great. Thanks!
Attachment #734406 -
Attachment is obsolete: true
Assignee | ||
Comment 26•8 years ago
|
||
Thanks! Looks like Linux and Mac OS X passed, but there was a try-server timeout on the windows platforms. Is there an easy way to requeue the build request?
Comment 27•8 years ago
|
||
Yep. I have retriggered it.
Assignee | ||
Updated•8 years ago
|
Attachment #734970 -
Flags: review?(mbrubeck)
Assignee | ||
Updated•8 years ago
|
Attachment #734261 -
Flags: review?(mbrubeck)
Assignee | ||
Comment 28•8 years ago
|
||
Looks like the try succeeded. Again, the errors in component/satchel look unrelated since relativesrcdir was not changed in that directory. Matt, since you landed the similar patch for bug 854868, I thought you might be a good person to review.
Comment 29•8 years ago
|
||
Comment on attachment 734261 [details] [diff] [review] dbaron's other mochitest patch from comment 5. Review of attachment 734261 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me, though it might be good to get a review from peers of each of the modules these live in -- or at least alert them, since it'll mean a minor workflow change for developers of those modules. Requesting feedback from some module peers.
Attachment #734261 -
Flags: review?(mbrubeck)
Attachment #734261 -
Flags: review+
Attachment #734261 -
Flags: feedback?(khuey)
Attachment #734261 -
Flags: feedback?(dtownsend+bugmail)
Attachment #734261 -
Flags: feedback?(bsmith)
Comment 30•8 years ago
|
||
Comment on attachment 734970 [details] [diff] [review] Fix hardcoded path names in mochitests to match relativesrcdir. Review of attachment 734970 [details] [diff] [review]: ----------------------------------------------------------------- Thanks a *ton* for doing this! Again, all these changes look correct to me, but I'm flagging some module peers to alert them to the changes and give them a chance to review if they choose.
Attachment #734970 -
Flags: review?(mbrubeck)
Attachment #734970 -
Flags: review+
Attachment #734970 -
Flags: feedback?(khuey)
Attachment #734970 -
Flags: feedback?(dtownsend+bugmail)
Attachment #734970 -
Flags: feedback?(dbaron)
Attachment #734970 -
Flags: feedback?(bsmith)
Comment 31•8 years ago
|
||
(In reply to Ben Kelly from comment #28) > Looks like the try succeeded. Again, the errors in component/satchel look > unrelated since relativesrcdir was not changed in that directory. We should make sure we can get a Try push with green on all platforms before landing these patches. The Windows satchel failures are suspicious because they are not intermittent and they did not show up on the parent changeset: https://tbpl.mozilla.org/?rev=b1fb34b07c17 I don't see anything suspicious in the failing test (test_bug_511615.html); it's also possible behavior has changed in some earlier test that is leaving things in a bad state. The test_xulbrowser_plugin_visibility.xul failure on Fedora64 also appears to be new; I retriggered to see if it's intermittent or not.
Assignee | ||
Comment 32•8 years ago
|
||
Thanks Matt! I agree the satchel tests are concerning. Unfortunately I haven't had any luck triggering these errors on my Mac and I don't have a windows dev environment at the moment. I did look through it at one point and didn't see any obvious paths that needed to be fixed. I didn't consider cross-test state issues, though.
Assignee | ||
Comment 33•8 years ago
|
||
Found another path issue with the test case failing on Fedora. It was a linux only test case. (Thanks for prodding me to go back and look at the platform specific failures!) I still haven't figured out or reproduced the satchel errors on windows. Does anyone have a windows environment that can reproduce this?
Attachment #734970 -
Attachment is obsolete: true
Attachment #734970 -
Flags: feedback?(khuey)
Attachment #734970 -
Flags: feedback?(dtownsend+bugmail)
Attachment #734970 -
Flags: feedback?(dbaron)
Attachment #734970 -
Flags: feedback?(bsmith)
Comment 34•8 years ago
|
||
I can try this locally on Windows, though I might not get to it until sometime next week.
Comment 35•8 years ago
|
||
toolkit/components/satchel/test/test_bug_511615.html succeeds when I run it alone, but fails on my Windows 8 laptop just like it did on Try when I run: _virtualenv/Scripts/python _tests/testing/mochitest/runtests.py --total-chunks=5 --this-chunk=5 --chunk-by-dir=4 --autorun --console-level=INFO I didn't get any more clue as to why it's failing. I guess I can try running it with differently-sized chunks of tests to narrow down which prior test(s) might be causing the failure.
Comment 36•8 years ago
|
||
If I change to "--total-chunks=6 --this-chunk=6" the test succeeds. The difference is that these tests directories are no longer included: /layout/forms/test/ /layout/generic/test/ /layout/inspector/tests/ /layout/mathml/tests/ None of these directories were touched directly by either patch here. Maybe some test in these directories depends on a file from /layout/base/tests or some other directory that was moved by these patches.
Comment 37•8 years ago
|
||
No file in these directories seems to refer to anything in any of the changed directories, unless I'm missing something. Maybe some the change in order of the tests has just exposed an existing issue in test_bug_511615.html -- perhaps the same one that previously showed up intermittently as bug 542580. I guess we can try attachment 505706 [details] [diff] [review] from that bug next...
Assignee | ||
Comment 38•8 years ago
|
||
Thanks for continuing the investigation on this! I really need to setup a windows environment. What about simply increasing the timeout between steps from 50ms to something like 250ms? Obviously not a long term solution, but if that resolves the failures then it would suggest the issue is due to timing effects. Also, its probably unrelated, but I've been running into bug 851641 in satchel as well.
Comment 39•8 years ago
|
||
I tried re-running M5 with a updated version of the patch from bug 542580, but still got the same failure. :(
Comment 40•8 years ago
|
||
In the screenshot from the M5 timeouts on Try, the Firefox menu is open. I saw the same thing when those tests failed locally, and it looked like it had been left open by a previous test before the satchel tests started. I didn't see which previous test opened the menu.
Comment 41•8 years ago
|
||
toolkit/components/prompts/test/test_bug620145.html seems to leave the Firefox menu open in some situations -- for example when I run: mach mochitest-plain --keep-open toolkit/components/prompts/test/test_bug620145.html
Comment 42•8 years ago
|
||
There is an duplicate synthesizeMouse in one of the code paths here. On Windows this ends up hitting the Firefox Menu button, which is harmless for this test but can cause failures in later tests, as seen above. This fixes the failure in test_bug_511615.html on my local machine. Pushed all three latest patches to Try: https://tbpl.mozilla.org/?tree=Try&rev=122944cf05d1
Attachment #737066 -
Flags: review?(dolske)
Comment 43•8 years ago
|
||
Comment on attachment 737066 [details] [diff] [review] fix for test_bug620145.html Review of attachment 737066 [details] [diff] [review]: ----------------------------------------------------------------- Presumably this is hitting the Firefox button because ui.button0 is no longer in the document? I wonder if synthesizeMouse() should check for that... Probably not common enough to worry about?
Attachment #737066 -
Flags: review?(dolske) → review+
Assignee | ||
Comment 44•8 years ago
|
||
Wow, nice catch! I'm still trying to get a windows environment setup, but maybe this will be a clue for the other failures: (In reply to Matt Brubeck (:mbrubeck) from comment #36) > /layout/forms/test/ > /layout/generic/test/ > /layout/inspector/tests/ > /layout/mathml/tests/ I added some debug to the mochitest harness to print out the full test list. I took a diff of the test list before and after applying the patches. It appears that somehow the path changes result in /layout/forms/test being added to the mochitest-5 run. The rest of these layout tests were already in mochitest-5. The order of the tests did not change.
Assignee | ||
Comment 45•8 years ago
|
||
For what its worth, mochitest-5 now includes /layout/forms/test due to the way directories are chunked. The mochitest-5 target uses --chunk-by-dir=4 which means it only looks at the first 4 directories in the test path. As a result, renaming: security/ssl/bugs security/ssl/mixedcontent/ security/ssl/stricttransportsecurity To: security/manager/ssl/tests/mochitest/bugs security/manager/ssl/tests/mochitest/mixedcontent security/manager/ssl/tests/mochitest/stricttransportsecurity Causes the chunking algorithm to see security/manager/ssl/tests as a single test directory instead of the previous three unique directories. Having two fewer total directories then causes the chunking algorithm to move a test dir from mochitest-4 to mochitest-5 due to rounding differences.
Comment 46•8 years ago
|
||
Comment on attachment 737066 [details] [diff] [review] fix for test_bug620145.html https://hg.mozilla.org/integration/mozilla-inbound/rev/6155cad4bea3
Attachment #737066 -
Flags: checkin+
Updated•8 years ago
|
Assignee: nobody → ben
Status: NEW → ASSIGNED
Whiteboard: [leave open]
Updated•8 years ago
|
Attachment #734261 -
Flags: feedback?(bsmith) → feedback+
Assignee | ||
Comment 48•8 years ago
|
||
Comment on attachment 736121 [details] [diff] [review] Updated Fixes for hardcoded path names in mochitests to match relativesrcdir. Sorry, just realized attachment 737066 [details] [diff] [review] fixed both issues in the satchel directory. I thought there was still another problem to resolve. Resubmitting the latest patch for review/feedback.
Attachment #736121 -
Flags: review?(mbrubeck)
Attachment #736121 -
Flags: feedback?(khuey)
Attachment #736121 -
Flags: feedback?(dtownsend+bugmail)
Attachment #736121 -
Flags: feedback?(dbaron)
Attachment #736121 -
Flags: feedback?(bsmith)
Reporter | ||
Comment 49•8 years ago
|
||
Comment on attachment 736121 [details] [diff] [review] Updated Fixes for hardcoded path names in mochitests to match relativesrcdir. (In reply to Matt Brubeck (:mbrubeck) from comment #30) > Comment on attachment 734970 [details] [diff] [review] > Fix hardcoded path names in mochitests to match relativesrcdir. > > Review of attachment 734970 [details] [diff] [review]: > ----------------------------------------------------------------- > > Thanks a *ton* for doing this! Again, all these changes look correct to me, > but I'm flagging some module peers to alert them to the changes and give > them a chance to review if they choose. I'm obviously ok with this, given that I filed the bug. :-)
Attachment #736121 -
Flags: feedback?(dbaron) → feedback+
Updated•8 years ago
|
Attachment #736121 -
Flags: feedback?(bsmith) → feedback+
Updated•8 years ago
|
Attachment #734261 -
Flags: feedback?(dtownsend+bugmail) → feedback+
Updated•8 years ago
|
Attachment #736121 -
Flags: feedback?(dtownsend+bugmail) → feedback+
Assignee | ||
Comment 50•8 years ago
|
||
I ran another try: https://tbpl.mozilla.org/?tree=Try&rev=979663f54c28 Looks like there is now a failure in mochitest-4 in dom/tests/geolocation. There was a commit on 4/12, so perhaps another relatvesrcdir dependent bit of code was added.
Assignee | ||
Comment 51•8 years ago
|
||
This is the commit I was referring to from 4/12 that touched the geolocation mochitests: https://hg.mozilla.org/mozilla-central/rev/37b472c847b1
Assignee | ||
Comment 52•8 years ago
|
||
Same error apparently is a known error on Android: https://bugzilla.mozilla.org/show_bug.cgi?id=778256 I'm still waiting for my updated tree to build. Will have to investigate tomorrow evening. I don't see anything obviously related to the relativesrcdir.
Assignee | ||
Comment 53•8 years ago
|
||
Its also interesting that test_mozsettingsWatch.html succeeds while test_mozsettings.html fails. The code up until the failure point is identical. If this is a timing issue, it could be related to the when the geolocation tests run in the build. With these patches the tests move from the end of mochitest-3 to be first thing in mochitest-4.
Assignee | ||
Comment 54•8 years ago
|
||
The mozSettings error appears unrelated to the relativesrcdir changes. I can reproduce it and have submitted a workaround to bug 778256. I believe its a race condition that was previously being masked by its location in the mochitest build order.
Comment 55•8 years ago
|
||
(In reply to Ben Kelly from comment #53) > Its also interesting that test_mozsettingsWatch.html succeeds while > test_mozsettings.html fails. The code up until the failure point is > identical. That's fall off from bug 859554, which introduced a timing dependency on the Contacts and Settings tests. I just landed a fix in bug 862247.
Comment 56•8 years ago
|
||
(In reply to Reuben Morais [:reuben] from comment #55) …a fix that also needs to be applied to those geolocation tests as well. Is there a way we set prefs on the profile used by mochitests? There's no reason to replicate that dance in every test that uses a preffed off API.
Comment 57•8 years ago
|
||
(In reply to Reuben Morais [:reuben] from comment #56) > (In reply to Reuben Morais [:reuben] from comment #55) > …a fix that also needs to be applied to those geolocation tests as well. Is > there a way we set prefs on the profile used by mochitests? There's no > reason to replicate that dance in every test that uses a preffed off API. http://mxr.mozilla.org/mozilla-central/source/build/automation.py.in#460
Assignee | ||
Comment 58•8 years ago
|
||
I've opened bug 862666 to track the fixes to the geolocation tests.
Updated•8 years ago
|
Attachment #736121 -
Flags: review?(mbrubeck) → review+
Assignee | ||
Comment 59•8 years ago
|
||
Is it fair to request checkin at this point? There is still feedback? request out to khuey and bug 862666 is in inbound, but not merged to m-c. (Sorry, can't access irc at the moment to ask.) Would be nice to move this forward as it seems susceptible to bitrot since it touches so many directories. Thanks.
Keywords: checkin-needed
Attachment #734261 -
Flags: feedback?(khuey) → feedback+
Attachment #736121 -
Flags: feedback?(khuey) → feedback+
Updated•8 years ago
|
Attachment #734261 -
Flags: checkin+
Updated•8 years ago
|
Attachment #736121 -
Flags: checkin+
Comment 60•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e0af273404e https://hg.mozilla.org/integration/mozilla-inbound/rev/b1096811620f
Keywords: checkin-needed
Comment 61•8 years ago
|
||
Unfortunately, it looks like this might've introduced some Android Mochitest-8 test failures in /tests/security/manager/ssl/tests/ : https://tbpl.mozilla.org/php/getParsedLog.php?id=21935347&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=21935487&tree=Mozilla-Inbound Looks like the recent try runs here didn't include Android (which is generally reasonable) -- so even with those green Try runs, it's feasible that this is causing the Android M-8 issues. So: this probably needs a backout, but first I'm retriggering the orange M-8 runs and watching M-8 on the subsequent builds to see if they hit the same test-failure.
Comment 62•8 years ago
|
||
Actually, I just went ahead with the backout, since mbrubeck agreed that it's more likely that this is responsible for the M-8 oranges on its push than that they sporadically cropped up randomly. (In reply to Ryan VanderMeulen [:RyanVM] from comment #60) > https://hg.mozilla.org/integration/mozilla-inbound/rev/8e0af273404e Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/ee1ba342612b > https://hg.mozilla.org/integration/mozilla-inbound/rev/b1096811620f Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/d4ff0407496f
Comment 63•8 years ago
|
||
(while I was backing out, another Android M-8 job completed on the original TBPL run, which also suffered from the same problem: https://tbpl.mozilla.org/php/getParsedLog.php?id=21936395&tree=Mozilla-Inbound )
Assignee | ||
Comment 64•8 years ago
|
||
I think these are known issues, but the exclusion in android.json was not updated for the new path: http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/android.json#301 I'll look for the other modified paths in config files and make a patch.
Assignee | ||
Comment 65•8 years ago
|
||
Here is a patch updating the android.json test exclusions. A quick search of json files did not show any other effected paths. https://tbpl.mozilla.org/?tree=Try&rev=bb2dce856c25 Sorry for the breakage!
Assignee | ||
Comment 66•8 years ago
|
||
I also found a reference to the old "security/ssl" path here: http://mxr.mozilla.org/mozilla-central/source/security/nss/tests/pkcs11/netscape/suites/security/ssl/manifest.mn#24 Its unclear to me if this is related or not.
Assignee | ||
Comment 67•8 years ago
|
||
Comment on attachment 738827 [details] [diff] [review] Fix android test exclusions to match new relativesrcdir paths. Android M8 is green with this patch: https://tbpl.mozilla.org/?tree=Try&rev=bb2dce856c25 Running another full try to double check for other breakage: https://tbpl.mozilla.org/?tree=Try&rev=02dcc645361a
Attachment #738827 -
Flags: review?(mbrubeck)
Attachment #738827 -
Flags: feedback?(dholbert)
Updated•8 years ago
|
Attachment #738827 -
Flags: review?(mbrubeck) → review+
Comment 68•8 years ago
|
||
Comment on attachment 738827 [details] [diff] [review] Fix android test exclusions to match new relativesrcdir paths. This makes sense - thanks for the quick work on the fixup here! (In reply to Ben Kelly from comment #66) > I also found a reference to the old "security/ssl" path here: > > http://mxr.mozilla.org/mozilla-central/source/security/nss/tests/pkcs11/ > netscape/suites/security/ssl/manifest.mn#24 > > Its unclear to me if this is related or not. (Probably not. Generally, it's good to avoid touching security/nss -- it's an upstream library that we import periodically, so any local changes there will get clobbered on whenever we update from upstream. If you did hypothetically need to change something there, it'd be better to do it upstream. But in this case it sounds like you don't, which is good.)
Attachment #738827 -
Flags: feedback?(dholbert) → feedback+
Assignee | ||
Comment 69•8 years ago
|
||
Thanks for catching the problem! I'll let the full try run to completion and then throw the checkin-needed keyword back on in the morning (assuming there aren't new problems). Do we need the [leave open] in the whiteboard still? With a widespread change like this is it common to leave the bug open for a while until any issues have shaken out? Thanks again.
Comment 70•8 years ago
|
||
(In reply to Ben Kelly from comment #69) > Do we need the [leave open] in the whiteboard still? With a widespread > change like this is it common to leave the bug open for a while until any > issues have shaken out? Nope, it can be removed. (/me removes) Generally, bugs are closed when their patches make it to mozilla-central (whether by merging from inbound or by directly landing there). [leave open] is just a signal to keep sheriffs from auto-closing a bug when merging from inbound, in cases where a bug has more patches that haven't yet been landed anywhere. (Looks like mbrubeck added it here when pushing an early patch in comment 46, to keep this bug from getting closed when that patch was merged to central. It was appropriate then, but isn't needed anymore.)
Whiteboard: [leave open]
Assignee | ||
Comment 71•8 years ago
|
||
Thanks for the explanation. That makes sense. Sadly, I forgot to update my tree before kicking off that last try, so it hit the mozsettings failures in M4. Here's another try with after a fresh hg pull -u: https://tbpl.mozilla.org/?tree=Try&rev=983c03ae4092
Assignee | ||
Comment 72•8 years ago
|
||
The last try looks pretty green. I don't see any consistent errors. I'm not sure on the process for handling backed out commits, but these are the patches that need to be (re)applied: attachment 734261 [details] [diff] [review] attachment 736121 [details] [diff] [review] attachment 738827 [details] [diff] [review]
Keywords: checkin-needed
Comment 73•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/22ed2b874b4a https://hg.mozilla.org/integration/mozilla-inbound/rev/46280589c1dc https://hg.mozilla.org/integration/mozilla-inbound/rev/896d943683eb
Keywords: checkin-needed
Updated•8 years ago
|
Attachment #738827 -
Flags: checkin+
Comment 74•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/22ed2b874b4a https://hg.mozilla.org/mozilla-central/rev/46280589c1dc https://hg.mozilla.org/mozilla-central/rev/896d943683eb
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in
before you can comment on or make changes to this bug.
Description
•