722075 - testing/mochitest/tests/browser/Makefile.in has commented-out lines before $(NULL)

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Daniel Holbert [:dholbert]]

The file testing/mochitest/tests/browser/Makefile.in has some commented-out lines at the end of its test lists, before the final $(NULL)

IIRC, any commented-out lines in the middle of a Makefile's list of mochitests will end the list prematurely, right at that point -- so really we want the list-terminating $(NULL) to be *before* the commented-out lines.

(In this case it doesn't really matter because there aren't any actual tests after the commented-out chunk; still, we might as well fix this in case any tests are added/uncommented later on, and for style/consistency/correctness points).

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix

Tagging gavin for r?, since his name's at the top of the file.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 592431 [details] [diff] [review]
fix

I left it this way because it makes it easier to re-enable those tests (you don't need to move the $(NULL) around), and AFAICT there is no practical impact. I guess people don't do that enough for it to matter.

Comment 3

[Daniel Holbert [:dholbert]]

> AFAICT there is no practical impact

None right now, correct -- but if someone were to add some more tests between the commented-out chunk and the $(NULL) line, e.g. to maintain alphabetical ordering or consistency or something, then it'd be bad - those added tests would never be run.  So I think best-practice is to have $(NULL) before the commented-out lines, to make it clear where the list actually ends.

Thanks for the review! Landed:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/3c2a8aa90e06

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/3c2a8aa90e06