532176 - target-specific assignments in testing/testsuite-targets.mk won't mix with assignments from the make command line

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Jim Blandy :jimb]

testing/testsuite-targets.mk contains the following:

jstestbrowser: EXTRA_TEST_ARGS += --extra-profile-file=$(topsrcdir)/js/src/tests/user.js
jstestbrowser:
	$(call RUN_REFTEST,$(topsrcdir)/js/src/tests/jstests.list)
	$(CHECK_TEST_ERROR)

However, if the user sets EXTRA_TEST_ARGS on the make command line, the target-specific assignment gets ignored.  For example:

$ cat simple.mk
all: VAR += value_from_makefile
all:
	@echo $(VAR)
$ make -f simple.mk
value_from_makefile
$ make -f simple.mk VAR=value_from_command_line
value_from_command_line
$ 

One can use 'override VAR+=...', which will make things work:

$ cat simple.mk
all: VAR += value_from_makefile
all:
	@echo $(VAR)
$ make -f simple.mk
value_from_makefile
$ make -f simple.mk VAR=value_from_command_line
value_from_command_line
$ 

However, in the case of testsuite-targets.mk, it would seem simpler for RUN_REFTEST to just take another argument.

Comment 1

[(not currently active) Ted Mielczarek]

on IRC I suggested that we just change that to:
jstestbrowser:
    $(call RUN_REFTEST,$(topsrcdir)/js/src/tests/jstests.list,--extra-profile-file=$(topsrcdir)/js/src/tests/user.js)
    $(CHECK_TEST_ERROR)

And change the definition of RUN_REFTEST to something like:
RUN_REFTEST = rm -f ./$@.log && $(PYTHON) _tests/reftest/runreftest.py \
   $(SYMBOLS_PATH) $(EXTRA_TEST_ARGS) $(2) $(1) | tee ./$@.log

Comment 2

[Bob Clary [:bc] (inactive)]

Ok. Considering I didn't find the issue or come up with the fix, I'm reluctant to take the bug, but can test it if someone else wants to take it.

Comment 3

[Jim Blandy :jimb]

Attachment: Fix 'make jstestbrowser EXTRA_TEST_ARGS=...'; allow command line to override test list.

Comment 4

[Bob Clary [:bc] (inactive)]

Comment on attachment 415522 [details] [diff] [review]
Fix 'make jstestbrowser EXTRA_TEST_ARGS=...'; allow command line to override test list.

I like the no change to the RUN_REFTEST and the ability to specify a list relative to topsrcdir via TEST_PATH. Thanks jimb.

Comment 5

[(not currently active) Ted Mielczarek]

Oh hah, that's even easier than what I wrote.

Comment 6

[Jim Blandy :jimb]

http://hg.mozilla.org/tracemonkey/rev/88a2ea2b2ca1

Comment 7

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/88a2ea2b2ca1