Microformats mochitests Makefile may have been overlooked in bug 370750

RESOLVED FIXED in Firefox 16

Status

()

Firefox
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dholbert, Assigned: froydnj)

Tracking

unspecified
Firefox 16
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

The microformats mochitest Makefile adds its mochitests by setting _TEST_FILES, as was the standard practice up until bug 370750 near-globally renamed _TEST_FILES to MOCHITEST_FILES.

Somehow, the microformats Makefile was missed, as it still says _TEST_FILES:
  https://mxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/Makefile.in#22

That variable almost certainly wants to be renamed to MOCHITEST_FILES.
Depends on: 370750, 384186
(In reply to Daniel Holbert [:dholbert] from comment #0)
> https://mxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/
> Makefile.in#22

er, pasted the wrong URL there (to a different makefile which may or may not have also been missed -- I'm not sure if makefiles in /testing/mochitest/ were intentionally skipped)

The microformats one is:
 https://mxr.mozilla.org/mozilla-central/source/toolkit/components/microformats/tests/Makefile.in#14
I suspect this needs the other tweaks from  bug 370750, too -- rules.mk moving, the "libs" lines being removed, etc.

froydnj, do you know if there's a reason this makefile was skipped?
Summary: Microformats mochitests were orphaned by bug 370750 → Microformats mochitests Makefile may have been overlooked in bug 370750
(Assignee)

Comment 3

5 years ago
Created attachment 641175 [details] [diff] [review]
patch

(In reply to Daniel Holbert [:dholbert] from comment #2)
> I suspect this needs the other tweaks from  bug 370750, too -- rules.mk
> moving, the "libs" lines being removed, etc.
> 
> froydnj, do you know if there's a reason this makefile was skipped?

My automated rewriting was looking for mochitests/tests/$(relativesrcdir) or similar, which this Makefile doesn't use, and my eyeballing of libs:: matches to catch stragglers overlooked this one.  Sorry about that!
Attachment #641175 - Flags: review?(mh+mozilla)
Attachment #641175 - Flags: review?(mh+mozilla) → review+
No worries, thanks for fixing!

Some other candidate Makefiles:
http://mxr.mozilla.org/mozilla-central/search?string=^_TEST_FILES&regexp=1&find=Makefile.in&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central

(The two 'identity' Makefiles in that list were just added yesterday in bug 753239, and it looks like they need fixing -- I posted over on that bug.)
Would it be possible/prudent to fail (tests or build) if a mochitest Makefile uses the old format?

I think we need either that, or repeated manual checking over the next few months, to be sure that no one accidentally lands patches with the old format (which they may have started working on back when the old format was still good -- as was the case in bug 753239).
(Assignee)

Comment 6

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/5ed247b1725f

(In reply to Daniel Holbert [:dholbert] from comment #5)
> Would it be possible/prudent to fail (tests or build) if a mochitest
> Makefile uses the old format?
> 
> I think we need either that, or repeated manual checking over the next few
> months, to be sure that no one accidentally lands patches with the old
> format (which they may have started working on back when the old format was
> still good -- as was the case in bug 753239).

Complaining loudly would be the best option, but I don't think it's feasible; _TEST_FILES and friends tend to be defined after rules.mk, so there's no central place to yell about _TEST_FILES &co.  Maybe a special libs:: rule that checks for _TEST_FILES &co. and yells?  Not sure if that would work...

I've already patched a few places that got missed in bug 370750; I'm happy to keep an eye out over the next month or two.
Assignee: nobody → nfroyd
Status: NEW → ASSIGNED
OS: Linux → All
Hardware: x86_64 → All
https://hg.mozilla.org/mozilla-central/rev/5ed247b1725f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16
You need to log in before you can comment on or make changes to this bug.