Skip to content

refactor: Remove private frameworks from the list of imports #596

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 24 commits into from
Jul 14, 2022

Conversation

mykola-mokhnach
Copy link

@mykola-mokhnach mykola-mokhnach commented Jul 12, 2022

This PR removes the explicit dependency to XCTAutomationSupport private framework because of the resends described in appium/appium#17174

The removal of the framework forces us to also remove all dependant classes, including XCElementSnapshot one. This PR replaces these dependant classes with interfaces. The only issue that ObjC does not allow to use default method implementations in protocol definitions, so we either need to introduce global helper methods for custom properties/extension methods or have a custom wrapper over XCElementSnapshot instances each time we need to call a customized entity on it. I've selected the second approach.

@mykola-mokhnach
Copy link
Author

it looks like the most complicated part there would be to make NSPredicate work properly with custom attributes. I knew it was going to explode some day.

Basically we need to parse the initial predicate expression as a string and transform it into a block, where we could process custom snapshot properties (e.g. these starting with wd...)

@Dan-Maor
Copy link
Collaborator

@mykola-mokhnach we can instead use class_addMethod in runtime for XCElementSnapshot and add our attributes.
I've tested it in a quick and dirty fashion by exposing wdLabel and isWDVisible from XCUIElement+FBWebDriverAttributes and added the following code to FBXCElementSnapshotWrapper:

+ (void)load
{
  Class XCElementSnapshotCls = objc_lookUpClass("XCElementSnapshot");
  
  // for wdLabel
  Method wdLabelMethod = class_getInstanceMethod([FBXCElementSnapshotWrapper class], @selector(wdLabel));
  class_addMethod(XCElementSnapshotCls, @selector(wdLabel), method_getImplementation(wdLabelMethod), method_getTypeEncoding(wdLabelMethod));
  
  // for isWDVisible
  Method isWdVisibleMethod = class_getInstanceMethod([FBXCElementSnapshotWrapper class], @selector(isWDVisible));
  class_addMethod(XCElementSnapshotCls, @selector(isWDVisible), method_getImplementation(isWdVisibleMethod), method_getTypeEncoding(isWdVisibleMethod));
}

For isWDVisible I also had to modify it to the following (since fb_isVisible is not defined for XCElementSnapshot, but on second thought we can also add this method directly as well as above):

- (BOOL)isWDVisible
{
  return [[FBXCElementSnapshotWrapper alloc] initWithSnapshot:self].fb_isVisible;
}

@mykola-mokhnach mykola-mokhnach changed the title [WIP] refactor: Remove private frameworks from the list of imports refactor: Remove private frameworks from the list of imports Jul 13, 2022
Copy link
Member

@KazuCocoa KazuCocoa left a comment

Choose a reason for hiding this comment

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

lg. I've checked a few with a real device

@KazuCocoa
Copy link
Member

What about the below fastlane version?
https://dev.azure.com/AppiumCI/Appium%20CI/_build/results?buildId=20298&view=logs&j=b0b7e611-f24f-5025-cfe1-d0e5eed5ba7f&t=1db81a0e-a352-5921-8e54-3d6dffa5f477

Fetching fastlane 2.162.0
Installing fastlane 2.162.0

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@KazuCocoa
Copy link
Member

let me try 2.162.0...

@mykola-mokhnach
Copy link
Author

Thanks @KazuCocoa
I'd say we are happy with this version for now + Ruby2 and stay with it until possible.

@abdulowork
Copy link

abdulowork commented Sep 9, 2022

One way we found to link with XCTAutomationSupport was to use a fake dylib with a name from the allowed names and reexport XCTAutomationSupport through -reexport_framework/-reexport_library linker flag. I made an example here.

A bit of a hack but maybe that's something that you would want to employ?

@pwfcurry
Copy link

pwfcurry commented Jan 3, 2023

Thanks for the fix @mykola-mokhnach!

Considering appium 2 is in beta, are there any plans to fix this for appium 1? (it still pulls in appium-webdriveragent v3, fix is in v4)

(couple of workarounds: use your old ~/Library/Developer/Xcode/DerivedData/WebDriverAgent-... dir built with xcode 13 (rename to match expected), or pin v4 of appium-webdriveragent)

@jlipps
Copy link
Member

jlipps commented Jan 3, 2023

@pwfcurry no, appium 1 is now unsupported. appium 2 is only in beta because the documentation hasn't been ready for a release. but in terms of its suitability for production use code-wise, it has been our only supported version for some time now. so it would make sense to update to appium 2 to take advantage of any fixes.

@KazuCocoa KazuCocoa deleted the xcode_upd branch September 24, 2023 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants