Enable compiled code tests in content/canvas

RESOLVED FIXED in mozilla19

Status

()

defect
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: bjacob, Assigned: bjacob)

Tracking

unspecified
mozilla19
x86_64
Linux
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

In Bug 732660, we added a first compiled-code test in a new directory, content/canvas/test/compiled.

We ran into the problem that they were being linked before libxpcom was linked, which caused linking to fail.

At that time, it seemed that replacing TEST_DIRS by TOOLS_DIRS in the Makefile was a way to solve this issue.

However, today I realized that that caused the content/canvas mochitests to not be run at all.

So what is the real way to enable compiled-code tests there? If TOOLS_DIRS is really the way, then that means that we should take compiled/ out of content/canvas/test.
Seems like TOOLS_DIRS was just a typo for TOOL_DIRS *sigh*

https://tbpl.mozilla.org/?tree=Try&rev=22f197df1767
Help needed to understand this cppunittest failure!
There's some bad error handling in the Python script there, but it sounds like it's probably failing to load some DLL that that test requires. I changed how C++ tests were run when I landed this runcppunittests.py script, they now get run from the directory they were built in, but with LD_LIBRARY_PATH or PATH set to point to dist/bin so they can load the rest of Gecko (that's what the --xre-path environment variable is for).

You can try running the test manually with something like:
PATH=e:/builds/moz2_slave/try-w32-dbg/build/obj-firefox/dist/bin:$PATH MOZ_XRE_DIR=e:/builds/moz2_slave/try-w32-dbg/build/obj-firefox/dist/bin e:/builds/moz2_slave/try-w32-dbg/build/obj-firefox/content/canvas/test/compiled/TestWebGLElementArrayCache.exe

If that doesn't give you any more information you can try running it under something like Dependency Walker: http://www.dependencywalker.com/ which should be able to show you what the error is.
This moves the compiled tests to a separate directory alongside test/ . So we no longer have both mochitests and compiled tests under the same directory. This seems to work, at least it built on Windows.

https://tbpl.mozilla.org/?tree=Try&rev=9a98775c1681
Attachment #670673 - Attachment is obsolete: true
Attachment #674733 - Flags: review?(ted)
Adapts a makefile to this directory move. Explains above build failures. New try:
https://tbpl.mozilla.org/?tree=Try&rev=909b464a40d2
Attachment #674733 - Attachment is obsolete: true
Attachment #674733 - Flags: review?(ted)
Attachment #675839 - Flags: review?(ted)
You need to wrap the TOOL_DIRS in  ifdef ENABLE_TESTS
Thanks. Also, the above try run discovered a real issue in the code, thanks to Windows' rand() function returning different values than Linux's with the same seed. Looking into it. Upping the number of repeats allows to reproduce on Linux.
Attachment #675839 - Flags: review?(ted)
See comment in the patch. The fix here was quite obvious from reproducing this in GDB. we were clamping lastByte but weren't clamping firstByte. Rather than clamping firstByte, a more general fix was to add this early-return path in the no-op case where firstByte > lastByte after clamping lastByte.
Attachment #675864 - Flags: review?(jgilbert)
Thanks Ms2ger for the tip.
Attachment #675839 - Attachment is obsolete: true
Attachment #675865 - Flags: review?(ted)
Question: if I change something in content/canvas/compiledtest/TestWebGLElementArrayCache.cpp and make -C objdir/content/canvas/compiledtest, it doesn't rebuild the test, it only re-runs it. That's a bit annoying. make -C objdir/content/canvas does rebuild the test. What's wrong and how can I get make -C compiledtest to rebuild the test?
Another question: the Android logs in comment 11 suggest that the test wasn't actually run on Android. It was run on Windows. What's wrong?
Oh, I suppose that as on Android we cross-compile, we can't run the compiled tests as part of the build job. So how can I see Android compiled tests on TBPL?
Comment on attachment 675865 [details] [diff] [review]
use TOOL_DIRS, and move compiled tests to a separate directory

Review of attachment 675865 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/canvas/test/compiled/Makefile.in
@@ +11,5 @@
>  
>  include $(DEPTH)/config/autoconf.mk
>  
>  LOCAL_INCLUDES := \
> +    -I$(srcdir)/../src \

You could fix this to 2-space indent while you're here.
Attachment #675865 - Flags: review?(ted) → review+
(In reply to Benoit Jacob [:bjacob] from comment #12)
> Question: if I change something in
> content/canvas/compiledtest/TestWebGLElementArrayCache.cpp and make -C
> objdir/content/canvas/compiledtest, it doesn't rebuild the test, it only
> re-runs it. That's a bit annoying. make -C objdir/content/canvas does
> rebuild the test. What's wrong and how can I get make -C compiledtest to
> rebuild the test?

I thought we fixed this in bug 777379. Did that not actually fix it?

(In reply to Benoit Jacob [:bjacob] from comment #14)
> Oh, I suppose that as on Android we cross-compile, we can't run the compiled
> tests as part of the build job. So how can I see Android compiled tests on
> TBPL?

We don't currently run C++ unit tests on Android. They're only run as part of "make check" on non-cross-compiled platforms.
Attachment #675864 - Flags: review?(jgilbert) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/d7d0cb3b68e0
https://hg.mozilla.org/integration/mozilla-inbound/rev/a31d3b784446
Assignee: nobody → bjacob
Summary: Enable compiled code tests in content/canvas/test/compiled → Enable compiled code tests in content/canvas
Target Milestone: --- → mozilla19
https://hg.mozilla.org/mozilla-central/rev/d7d0cb3b68e0
https://hg.mozilla.org/mozilla-central/rev/a31d3b784446
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Oops, the "increase test repeats" part failed to apply and i hadn't noticed. Good thing I checked test logs.

https://hg.mozilla.org/integration/mozilla-inbound/rev/d33f050f2f34
You need to log in before you can comment on or make changes to this bug.