231 bytes, text/plain
6.33 KB, patch
Benjamin Smedberg: review+
|Details | Diff | Splinter Review|
151.52 KB, patch
|Details | Diff | Splinter Review|
Releng hit a problem while building mozilla-central on windows. The error message is: Could not create the directory: ..\..\..\..\..\..\_tests\testing\mochitest\tests\parser\htmlparser\tests\mochitest\html5lib_tree_construction\scripted: No such file or directory It would be great to shorten the path of mochitest.
Created attachment 495060 [details] shorteh.sh A simple bash script replacing hardcoded paths. It scans Makefile.in files only. Other 2 files will be patched in the next patch.
Created attachment 495061 [details] [diff] [review] Manual fixes
Created attachment 495425 [details] [diff] [review] Manual fixes Files under testing/mochitest require additional intervention, I realized after a try build. * js/src/config/config.mk synced with config/config.mk * Adjusted relativesrcdir for files in testing/mochitest This patch should be applied after running shorten.sh There are 6 oranges, hopefully unrelated to the changes made by the patch. It would be great to recheck this. Here is a summary of the oranges: http://pastebin.mozilla.org/877061
rail, I think your two Manual Fixes patches are disjoint and we would need them both? am I wrong?
Created attachment 495461 [details] [diff] [review] Manual fixes (In reply to comment #4) > rail, I think your two Manual Fixes patches are disjoint and we would need them > both? am I wrong? Oh, yeah. The result of staying too late. :) Thanks for the catch.
Comment on attachment 495461 [details] [diff] [review] Manual fixes >--- a/testing/testsuite-targets.mk >+++ b/testing/testsuite-targets.mk >@@ -48,17 +48,17 @@ endif > SYMBOLS_PATH := --symbols-path=$(DIST)/crashreporter-symbols > > # Usage: |make [TEST_PATH=...] [EXTRA_TEST_ARGS=...] mochitest*|. > MOCHITESTS := mochitest-plain mochitest-chrome mochitest-a11y mochitest-ipcplugins > mochitest:: $(MOCHITESTS) > > RUN_MOCHITEST = \ > rm -f ./$@.log && \ >- $(PYTHON) _tests/testing/mochitest/runtests.py --autorun --close-when-done \ >+ $(PYTHON) $(mochitestdir)/runtests.py --autorun --close-when-done \ > --console-level=INFO --log-file=./$@.log --file-level=INFO \ > $(SYMBOLS_PATH) $(TEST_PATH_ARG) $(EXTRA_TEST_ARGS) > > ifndef NO_FAIL_ON_TEST_ERRORS > define CHECK_TEST_ERROR > @errors=`grep "TEST-UNEXPECTED-" $@.log` ;\ > if test "$$errors" ; then \ > echo "$@ failed:"; \ >@@ -135,16 +135,17 @@ xpcshell-tests: > --manifest=$(DEPTH)/_tests/xpcshell/all-test-dirs.list \ > --no-logfiles \ > $(SYMBOLS_PATH) \ > $(TEST_PATH_ARG) $(EXTRA_TEST_ARGS) \ > $(DIST)/bin/xpcshell > > # Package up the tests and test harnesses > include $(topsrcdir)/toolkit/mozapps/installer/package-name.mk >+include $(topsrcdir)/config/config.mk This config.mk include seems unnecessary. (We clearly already use a bunch of stuff from config.mk in this file.) Despite being a large scary invasive change, this seems ok.
(In reply to comment #6) > Despite being a large scary invasive change, this seems ok. Ted Mielczarek, to resolve this bug we need only to delete that line from the patch? This bug is preventing my build to work, so I would like to see it resolved.
Yes, with just that change we should be able to land this. Does this patch still apply to the latest source?
I can't build Firefox because of the same problem reported by Rail Aliiev, so I think it still applies.
This bug is also for Windows 7 64 bit, not only for Linux.
Created attachment 552904 [details] [diff] [review] Manual fixes corrected Removed the useless line.
Marco, there's something up with the patch as it doesn't import without a parse error into mercurial. I tried to fix it up, but it then still doesn't apply cleanly to tip - and manually resolving made me notice new instances (eg RUN_MOCHITEST_REMOTE) that need shortening now as well. Also, the diff of attached patch vs my fixed up patch is huge, which makes me a bit uneasy. The attached patch didn't have the full amount of context, which is probably the main reason why, but I don't have the time to pick through and check that something didn't break during fixing it up. Please can you: - check for new instances to shorten - update to tip - add author and commit message - make sure the patch isn't malformed - use the format/context settings here: http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed - send to try ...and then re-request checkin-needed. Thanks :-)
Created attachment 567284 [details] [diff] [review] Shorten mochitest paths for m-c, V4 I'm not sure if Rail/Marco was going to work on this but I happened to refresh the patch while I was introducing myself to mochitests. Patch is updated to tip and should handle current mochitests in m-c (there seems to have been a lot new tests created during last year!) as I run the attached shorteh.sh script and also verified that _tests/testing directory no longer contains anything (that's the old location) after a build. I tested running Windows mochitests locally and at least that started and run the tests through. Additionally this patch removes the _mochitest directory at the same time _tests directory is removed for rebuilds. This way it should match the old behaviour and make sure automationutils.py and tests are refreshed without clobbering builds manually all the time. I don't have try-server access so would be nice if somebody could run the patch through try for me, thanks!
Pushed to try, but the specialpowers changes landed in the last day or so have bitrotted the patch in a few places. Did the best I could to fixed locally, but not sure if I got everything: https://tbpl.mozilla.org/?tree=Try&rev=a69d2e0bfd28
Created attachment 567546 [details] [diff] [review] Shorten mochitest paths for m-c, V4.1 Thanks for the try-run. I've fixed the one Moth failure that seems to have been due to two new DOM Worker test extensions makefiles and also otherwise unrotted this version.
Comment on attachment 567546 [details] [diff] [review] Shorten mochitest paths for m-c, V4.1 Review of attachment 567546 [details] [diff] [review]: ----------------------------------------------------------------- I didn't look at every single line of this patch, but I reviewed this in the past so I'm just going to r+ this. I did spot check a bunch of places and they look fine.
Created attachment 567894 [details] [diff] [review] Shorten mochitest paths for c-c, V4.1 This is the required changes in the comm-central repo.
Comment on attachment 567894 [details] [diff] [review] Shorten mochitest paths for c-c, V4.1 If this was tested on c-c (not suite has horrid orange anyway) I give this patch a+ even if SeaMonkey tree is still closed, so we can avoid bitrot
Created attachment 570627 [details] [diff] [review] Shorten mochitest paths for m-c, V4.2 Simple refresh due to recent code suffling. Carrying over r=ted since changes are minimal (or try to).
Rail did all the heavy lifting here. I did run c-c suite tests locally and they did run with only few failures.
@ted: I have not enough privileges to land the patch to m-i/m-c. Could you land it?
Comment on attachment 570627 [details] [diff] [review] Shorten mochitest paths for m-c, V4.2 This didn't apply cleanly to inbound, but I've fixed locally & added the missing author/commit message (for future patches, please see http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed). Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=9bcd060f8d1b Thanks :-)
Created attachment 572084 [details] [diff] [review] m-c part followup dom/battery/test/Makefile.in was created since the initial patch, so needs changing too: http://mxr.mozilla.org/mozilla-central/source/dom/battery/test/Makefile.in#59
I am having problems executing the mochitests: http://pastebin.mozilla.org/1374664 If I do make -C ff-obj mochitest-browser-chrome it tells me firefox-bin doesn't exist. Why does the script look for the firefox-bin in the src folder instead of the ff-obj directory? I tried running the mochitests from the root src folder and from the ff-obj dir. Same error.
ditto what mihai said in comment #26.
why are we landing massive build system changes on the weekend before a merge?
Backed out preemptively in case this was the cause (and yet works on try/m-c somehow), given that if it is, it's stopping people test locally leading up to the merge: https://hg.mozilla.org/mozilla-central/rev/e697147121b3
Comment on attachment 572084 [details] [diff] [review] m-c part followup Is obsolete, as will be added to the main patch when it relands.
confirmed that this was the cause of the problem. Please wait until after Tuesday (November 8th) before relanding this. We can deal with breakage then. thanks.
Thanks for confirming Rob, I'll leave it to Rail/wicked to fix, test locally and re-request review from ted.
I'm not sure if I'm the best person to be an assignee. :( Back to the pool of Core/Build Config.
Rail, I assigned this to you to mark the fact that you did the initial/most work (and thus should get credit for fixing this, not sure if you follow that principle, though) but I guess that'll work better when this is actually fixed. :) Anyway, I'll update the patch as soon as I figure out what's wrong.
Teemu, I just wanted to keep my "Assigned bugs" list shorter :) and make sure that Core/Build Config "unassigned bugs" (tirage?) query lists this bug. Thanks a lot for your hard work keeping the patch up to date and testing it!
Actually, I doubt I'll get to this ever but this seems to be taken care of slowly by other bugs anyway..