Skip to content

Conversation

@pabelnl
Copy link

@pabelnl pabelnl commented Oct 14, 2019

Includes:

  • PocketNetworkClient: Public class that conforms to the NetworkClient protocol, adding the necessary steps and information to interact with the Pocket Network.

  • Carthage dependency: Added pocket-swift-core-carthage to the carfile, necessary to use all the core functionalities for Pocket.

  • Updated Project: Added necessary frameworks to the carthage script section and to the test target copy files to properly run the tests.

@keefertaylor keefertaylor self-requested a review October 16, 2019 00:22
@keefertaylor
Copy link
Owner

Can you provide the commands you're using to build? Running integration tests in Xcode gives me:

The bundle “IntegrationTests” couldn’t be loaded because it is damaged or missing necessary resources. Try reinstalling the bundle.
2019-10-15 20:41:32.976364-0400 xctest[28938:2344211] (dlopen_preflight(/Users/keefertaylor/Library/Developer/Xcode/DerivedData/TezosKit-azhwkmvjudwqolgugmsphxlkimqx/Build/Products/Debug-iphonesimulator/IntegrationTests.xctest/IntegrationTests): Library not loaded: @rpath/RNCryptor.framework/RNCryptor
  Referenced from: /Users/keefertaylor/Library/Developer/Xcode/DerivedData/TezosKit-azhwkmvjudwqolgugmsphxlkimqx/Build/Products/Debug-iphonesimulator/TezosKit.framework/Frameworks/PocketSwift.framework/PocketSwift
  Reason: no suitable image found.  Did find:
	/Users/keefertaylor/Library/Developer/Xcode/DerivedData/TezosKit-azhwkmvjudwqolgugmsphxlkimqx/Build/Products/Debug-iphonesimulator/TezosKit.framework/Frameworks/PocketSwift.framework/Frameworks/RNCryptor.framework/RNCryptor: no matching architecture in universal wrapper)

I also have some failures in PocketNetwork unit tests.

Copy link
Owner

@keefertaylor keefertaylor left a comment

Choose a reason for hiding this comment

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

Thank you for taking the time to contribute to TezosKit. This PR overall looks great.

Here are a few notes that I think will help this integration remain easy to maintain and make it easy for clients to use. I don't feel incredibly strongly about these things, and I'm happy to take this PR as is if you'd rather not implement.


If you'd like, I'd be supportive of you adding an extension to TezosNodeClient to automatically make a NodeClient with the Pocket network attached. You could place in /Pocket/TezosNodeClient+Pocket and implement with:

extension PocketNetworkClient {
  public static pocketNetworkNodeClient() -> PocketNetworkClient {
    let pocketNEtworkClient = PocketNetworkClient(...)
    return new TezosNodeClient(networkClient: pocketNetworkClient, ....) 
  }
}

(You could also use a convenience initializer if you prefer to the static factory method.


If you'd like to have integration tests, you can feel free to add them. I run integration tests manually on merge.

@keefertaylor
Copy link
Owner

Here are the commands and resulting log files:

Unit Tests:

$ set -o pipefail && xcodebuild test -scheme TezosKit -destination 'platform=iOS Simulator,name=iPhone 8,OS=13.0' ONLY_ACTIVE_ARCH=YES | xcpretty

Output: https://gist.github.com/keefertaylor/717bbea39a9e788e049da0397019953f


Integration Tests:

$ set -o pipefail && xcodebuild test -scheme IntegrationTests -destination 'platform=iOS Simulator,name=iPhone 8,OS=13.0' ONLY_ACTIVE_ARCH=YES | xcpretty

Output: https://gist.github.com/keefertaylor/c4db31c1b82b535476064afd08ed491c

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.

2 participants