Closed Bug 816822 Opened 8 years ago Closed 8 years ago

Android build patches for WebRTC unit tests

Categories

(Core :: WebRTC, defect, P2)

ARM
Android
defect

Tracking

()

RESOLVED FIXED
mozilla20

People

(Reporter: dmose, Assigned: gcp)

References

Details

(Whiteboard: [WebRTC] [blocking-webrtc-] [qa-])

Attachments

(1 file, 3 obsolete files)

No description provided.
I didn't get as much time to poke at this today as I was hoping.

This patch is a hack that makes the signaling tests build; my suspicion is that the right fix there will not touch gtest-port.h but will do the appropriate override somewhere at a higher level.

Still need to fix the mtransport tests.
I think we can just do -DGTEST_HAS_TR1_TUPLE=0 on Android.
Blocks: 750869
Cleaner fix for dmose's issue. Also fixes all remaining issues.

IIRC in one of the earlier patches I made Android set Userspace_os_Linux and added ifdef ANDROID's, this code is doing the opposite. Should probably clean this up to make all patches uniform.
Attachment #686909 - Attachment is obsolete: true
Something on mozilla-inbound caused this patch to bit-rot.  Here's an updated version that applies.
Attachment #687116 - Attachment is obsolete: true
Flags: needinfo?
Attachment #687931 - Flags: review?(dmose)
Attachment #687931 - Flags: feedback?(ted)
Priority: -- → P2
Whiteboard: [WebRTC] [blocking-webrtc-]
Comment on attachment 687931 [details] [diff] [review]
Patch 1, v3, tests to make C++ unit tests build on Android

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

::: media/mtransport/test/sctp_unittest.cpp
@@ +97,5 @@
>      EXPECT_GE(r, 0);
>  
>      memset(&local_addr_, 0, sizeof(local_addr_));
>      local_addr_.sconn_family = AF_CONN;
> +#if !defined(__Userspace_os_Linux) && !defined(__Userspace_os_Windows) && !defined(__Userspace_os_Android)

As per before, maybe this wants to just be defined(__Userspace_os_Darwin) ?
Attachment #687931 - Flags: feedback?(ted) → feedback+
Flags: needinfo?
The old patch was bitrotted by some recent checkin.  Here's an unbitrotted version, with feedback+ and review? carried forward.
Attachment #687931 - Attachment is obsolete: true
Attachment #687931 - Flags: review?(dmose)
Attachment #691094 - Flags: review?(dmose)
Attachment #691094 - Flags: feedback+
Comment on attachment 691094 [details] [diff] [review]
Patch 1, v4, fixes to make C++ unit tests build on Android

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

This patch looks good, my only question is about the license on the nrappkit files.  The license doesn't actually state what license it is, which makes it harder to analyse.  

If these are just copies of other files in the tree, and licensing@mozilla.org has already OKed this license, r=dmose.  If not, can you either elaborate on why we don't need approval, or else mail them for approval?

r=dmose conditional on the licensing stuff being sorted out
Attachment #691094 - Flags: review?(dmose) → review+
Oh, this also still needs the tweak the Ted requested in comment 5 before it lands.
>If these are just copies of other files in the tree

They are copies of the already-present linux/ files, with small fixes for Android.
Assignee: nobody → gpascutto
OS: Mac OS X → Android
Hardware: x86 → ARM
Landed on mozilla-inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/5bbd7e9882b3

Setting in-testsuite-, because this is a build fix, so the compiler itself acts as an automated test.
Flags: in-testsuite-
https://hg.mozilla.org/mozilla-central/rev/5bbd7e9882b3
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Whiteboard: [WebRTC] [blocking-webrtc-] → [WebRTC] [blocking-webrtc-] [qa-]
You need to log in before you can comment on or make changes to this bug.