Closed Bug 1382750 Opened 3 years ago Closed 3 years ago

Run ots gtests

Categories

(Core :: Graphics: Text, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: dholbert, Assigned: jfkthame)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 file)

OTS comes with gtests to unit-test itself (which you can run independently via "make check")

We should probably wire those up to run with all of our other gtests.

(jfkthame, let me know if we already do this somehow -- but I don't think we do.  I looked at a recent Linux GTest log:
https://public-artifacts.taskcluster.net/Xe1vlRczTo2R-OtqAwFBnw/0/public/logs/live_backing.log
...and I didn't find any mentions of the strings "/ots" or "font" in that log.
)

I think this would involve editing the moz.build file (mozilla-specific build manifest) for ots:
https://dxr.mozilla.org/mozilla-central/rev/1b065ffd8a535a0ad4c39a912af18e948e6a42c1/gfx/ots/src/moz.build

...and extending TEST_DIRS there somehow -- or maybe DIRS? I'm not entirely sure -- I'm seeing TEST_DIRS in the source, but our documentation says DIRS:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/GTest#Adding_a_test_to_the_build_system

Anyway, here are some other pieces of code where we do this using TEST_DIRS (notably including some third party libraries like libcubeb, libmar, libstagefright, webrtc -- and ots would just be yet another 3rd-party library):
  https://dxr.mozilla.org/mozilla-central/search?q=TEST_DIRS
(Yeah, after looking more closely, I think DIRS is the right thing to append to -- not TEST_DIRS.  Though maybe we need some extra massaging to get things wired up correctly beyond that -- not sure.)
:jfkthamem, can you comment to the bug? Thanks!
Flags: needinfo?(jfkthame)
Daniel is right, we don't currently run these tests. In fact, we don't even have them in our tree, so we'd first need to add the /test/ dir to what we import from upstream (see the gfx/ots/sync.sh script).

This sounds like it'd be worth doing, though it's possible there will be some complications with getting them to run in our automation... I guess all we can do is try it, and see how it goes.
Flags: needinfo?(jfkthame)
Whiteboard: [gfx-noted]
Here's a patch that adds the gtest unit tests from upstream to our './mach gtest' run. I have deliberately kept OTS at our current revision when updating the import script to include the test files; we'll also want to take a new OTS version, but that should be done separately in bug 1348788.
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Try seems happy with this:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=673d7386973c86f1f6e693c392e87316ed3b6ad8

(The OTS tests begin with "ValidateTest.TestRMoveTo" in the log of a typical Gtest run there.)
Attachment #8890282 - Flags: review?(milan)
Comment on attachment 8890282 [details] [diff] [review]
Import and run gtests from upstream OTS

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

Nice!
Attachment #8890282 - Flags: review?(milan) → review+
https://hg.mozilla.org/mozilla-central/rev/a0c7afb1b839
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.