Closed Bug 851975 Opened 10 years ago Closed 10 years ago
_TESTS in backend .mk always false
Well, this is embarrassing. In backend.mk, we write out TEST_TOOL_DIRS as: ifdef ENABLE_TESTS TOOL_DIRS += <value> endif Unfortunately, backend.mk is included before config.mk. So, ENABLE_TESTS is *never* defined. So, essentially these directories have not been built since the moz.build switchover on February 28. Let's only hope there haven't been any regressions. https://mxr.mozilla.org/mozilla-central/search?string=TEST_TOOL_DIRS reveals affected files. Patch will come shortly.
This is the simplest solution. We keep backend.mk static. An alternative solution would be to move the include of backend.mk to after the inclusion of config.mk in rules.mk. If config.status changes, a moz.build regeneration will be triggered. Thus, if ENABLE_TESTS changes, backend.mk should be updated automagically. Broadcasting review to whoever gets it first. https://tbpl.mozilla.org/?tree=Try&rev=212b3c4d231d If we regressed tests while there wasn't test coverage, this could be very painful...
Comment on attachment 725950 [details] [diff] [review] Don't use ifdefs in backend.mk, v1 Review of attachment 725950 [details] [diff] [review]: ----------------------------------------------------------------- Ouch. :-/
Attachment #725950 - Flags: review?(ted) → review+
Pushed straight to central per philor's recommendation. https://hg.mozilla.org/mozilla-central/rev/a9a25781850e
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
OK, so this isn't as bad as I thought. This only has an impact if the Makefile.in was not including config.mk before rules.mk. I have yet to audit the full list, but /content/base/Makefile.in was definitely affected. (This is what caused the mochitest failure in bug 844635.)
Hmmm. I may have goofed here. I made it about halfway through auditing the Makefile.in seemingly affected by this and every one of them included autoconf.mk. Since autoconf.mk defines ENABLE_TESTS, this patch should have no effect. Since I was fooling around with bug 844635 when I filed this bug, I suspect the state of my tree had something to do with this. In that bug, the auto-generated Makefile don't include autoconf.mk before rules.mk. And if autoconf.mk isn't included, ENABLE_TESTS isn't defined and thus this bug. I suspect I ran a |make| against a tree with auto-generated Makefiles, noticed the 'tests' directory wasn't being traversed, looked at backend.mk, and concluded "oh crap, ENABLE_TESTS is never defined before backend.mk is evaluated." *sigh*. It happens. So, unless there was a Makefile.in not including autoconf.mk, all I did in this bug was fix the issue preventing bug 844635 from landing. Yay, I guess? So, in the words of Frank Drebin , "there's nothing to see here."  https://www.youtube.com/watch?v=5NNOrp_83RU
Try run for 212b3c4d231d is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=212b3c4d231d Results (out of 213 total builds): exception: 79 success: 123 warnings: 1 failure: 10 Builds (or logs if builds failed) available at: http://email@example.com
You need to log in before you can comment on or make changes to this bug.