Closed Bug 466405 Opened 13 years ago Closed 13 years ago

Ogg and Wave specific mochitests should be hidden by appropriate build-time conditional

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: kinetik, Assigned: kinetik)

Details

(Keywords: fixed1.9.1)

Attachments

(1 file, 2 obsolete files)

All of the mochitests specified in content/media/video/test/Makefile.in are used when MOZ_MEDIA is defined (the "video" directory is conditional on MOZ_MEDIA).  It's possible to build with the Ogg or Wave decoder disabled, so the tests specific to a backend should be conditional on that backend's build-time config.
Attached patch patch v1 (obsolete) — Splinter Review
Assignee: nobody → kinetik
Attachment #349689 - Flags: review?(ted.mielczarek)
Attachment #349689 - Flags: review?(ted.mielczarek) → review+
Comment on attachment 349689 [details] [diff] [review]
patch v1

Can you reformat this such that each test file is on its own line? Like:
+_TEST_FILES += \
   test_bug461281.html \
Attached patch patch v2 (obsolete) — Splinter Review
Done.  Also converted the spaces to tabs to bring it in line with the rest of our Makefiles.
Attachment #349689 - Attachment is obsolete: true
Keywords: checkin-needed
Comment on attachment 349821 [details] [diff] [review]
patch v2

>-                r11025_u8_c1.wav \
>-#                test_bug448534.html \
>-                $(NULL)
>+		seek.ogg \
>+#		test_bug448534.html \
>+		$(NULL)

This is only a "hack" if it works:
to be (future) safe, move the commented out line out/after of the list.
Attachment #349821 - Attachment is obsolete: true
I'm not sure what the problem is.  Can you please explain?  That line was commented out the same way before my patch, and commenting it out that way works fine.
Attached patch patch v3Splinter Review
Rebased against current tip.  Hopefully this'll get checked in ASAP so it doesn't bitrot, since just about any work on the media stuff adds new tests.
Note that this reenables a test for bug 448534 that was disabled in August because of random crashes.  It's likely that the cause of the crash has since been fixed, so we should try to reenable this test and see what happens.
Keywords: checkin-needed
Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/64d08af48174
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [baking for 1.9.1]
Whiteboard: [baking for 1.9.1] → [baking for 1.9.1][needs 191 landing]
Pushed db50c5ff1757 to 1.9.1
Keywords: fixed1.9.1
Whiteboard: [baking for 1.9.1][needs 191 landing]
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.