Closed Bug 851975 Opened 10 years ago Closed 10 years ago

ifdef ENABLE_TESTS in always false


(Firefox Build System :: General, defect, P1)



(Not tracked)



(Reporter: gps, Assigned: gps)


(Blocks 1 open bug)



(1 file)

Well, this is embarrassing.

In, we write out TEST_TOOL_DIRS as:

TOOL_DIRS += <value>

Unfortunately, is included before So, ENABLE_TESTS is *never* defined. So, essentially these directories have not been built since the switchover on February 28. Let's only hope there haven't been any regressions. reveals affected files. Patch will come shortly.
This is the simplest solution. We keep static.

An alternative solution would be to move the include of to after the inclusion of in

If config.status changes, a regeneration will be triggered. Thus, if ENABLE_TESTS changes, should be updated automagically.

Broadcasting review to whoever gets it first.

If we regressed tests while there wasn't test coverage, this could be very painful...
Assignee: nobody → gps
Attachment #725950 - Flags: review?(ted)
Attachment #725950 - Flags: review?(mh+mozilla)
Attachment #725950 - Flags: review?(khuey)
Comment on attachment 725950 [details] [diff] [review]
Don't use ifdefs in, v1

Review of attachment 725950 [details] [diff] [review]:

Ouch. :-/
Attachment #725950 - Flags: review?(ted) → review+
Attachment #725950 - Flags: review?(mh+mozilla)
Attachment #725950 - Flags: review?(khuey)
Pushed straight to central per philor's recommendation.
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 was not including before I have yet to audit the full list, but /content/base/ was definitely affected. (This is what caused the mochitest failure in bug 844635.)
Blocks: 846425
No longer blocks: 846825
Hmmm. I may have goofed here. I made it about halfway through auditing the seemingly affected by this and every one of them included Since 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 before And if 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, and concluded "oh crap, ENABLE_TESTS is never defined before is evaluated." *sigh*. It happens.

So, unless there was a not including, 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 [1], "there's nothing to see here."

Try run for 212b3c4d231d is complete.
Detailed breakdown of the results available here:
Results (out of 213 total builds):
    exception: 79
    success: 123
    warnings: 1
    failure: 10
Builds (or logs if builds failed) available at:
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.