Skip to content
This repository has been archived by the owner on Dec 26, 2019. It is now read-only.

Add tvOS support (dependencies and headers) #1032

Open
wants to merge 84 commits into
base: master
Choose a base branch
from

Conversation

shvul
Copy link

@shvul shvul commented Nov 19, 2018

PROBLEM


Add support for tvOS #640

  • update target dependencies for tvOS integration
  • update XCTest private header to work with Apple TV

SOLUTION


  • updated XCTest headers
  • update dependencies

NOTES


Tests results: https://travis-ci.org/shvul/WebDriverAgent/builds/456840661
WDA tests issue would be fixed in next pr: https://travis-ci.org/shvul/WebDriverAgent/builds/456857954

@@ -1 +1 @@
github "marekcirkos/RoutingHTTPServer" "v1.0.1"
github "shvul/RoutingHTTPServer" "v1.0.2"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly has been changed in this module that is has to be updated?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous version doesn't have tvOS target, so Carthage cannot build it for our purposes. Also, CocoaHTTPServer lib was updated. I added small fix for tvOS compiling.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say it makes sense to create a PR to update the original rep rather than switching to the custom one.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original repo has not been updated for a long time. @marekcirkos made a fork for his fixes. Do you suggest to make pr to his fork?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, this makes sense, because he is anyway the only person who could approve and merge this PR

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mykola-mokhnach here it is: marekcirkos/RoutingHTTPServer#1
Let's continue when @marekcirkos merge it and create new release

@@ -31,7 +31,9 @@
@property(getter=isIdleAnimationWaitEnabled) BOOL idleAnimationWaitEnabled; // @synthesize idleAnimationWaitEnabled=_idleAnimationWaitEnabled;
@property(nonatomic) BOOL doesNotHandleUIInterruptions; // @synthesize doesNotHandleUIInterruptions=_doesNotHandleUIInterruptions;
@property(readonly) BOOL fauxCollectionViewCellsEnabled;
#if !TARGET_OS_TV
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think TODO can be removed now

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<Scheme
LastUpgradeVersion = "0900"
LastUpgradeVersion = "1000"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I this necessary to update this file?

Copy link
Author

@shvul shvul Nov 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not necessary. As I remember, I just update repo to "recommended settings". If I revert its update, it will cause merge conflict in further pr's. So, if it is not a serious problem, I will leave this file as it is. It would be updated in next pr's.

@mykola-mokhnach
Copy link
Contributor

@shvul Feel free to recreate this PR to Appium's WDA fork if it takes too much time here to wait for a review from repository owners.

@shvul
Copy link
Author

shvul commented Nov 26, 2018

@mykola-mokhnach Just a quick note, that it would take some time to update other part of the tvOS logic to support Appium based features (if possible).

@mykola-mokhnach
Copy link
Contributor

@shvul Make it simple. I would rather merge the basic functionality we have now so then we can create more PRs and make it more advanced if needed

@g-tiwari
Copy link

g-tiwari commented Feb 7, 2019

Do we have any update on this integration ?
@shvul I would love to help if anything is needed.

@shvul
Copy link
Author

shvul commented Feb 8, 2019

@g-tiwari I'm not able to make all changes to merge with Appium fork due to a huge workload. Also, Appium version of WDA uses a lot of extra libs, that doesn't support tvOS. So, first of all you may try to add support to them by your own :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants