Skip to content
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

Add FirebaseAuth test_spec #3123

Merged
merged 13 commits into from
Jun 6, 2019
Merged

Add FirebaseAuth test_spec #3123

merged 13 commits into from
Jun 6, 2019

Conversation

paulb777
Copy link
Member

@paulb777 paulb777 commented Jun 1, 2019

  • Add unit test spec for Auth.
  • Add a complete Auth build and test rule.
  • Testing is done via the pod gen generated workspace.
  • AuthSample and its associated tests are migrated to their own isolated xcodeproj and associated Podfile.
  • I've verified that CI will build and run the integration tests, but they're currently disabled in .travis.yml because of backend issues.
  • Directory structure will be cleaned up in a future PR.
  • Fix unit test warnings.
  • Fix string in tests that failed in c99 builds.
  • Eliminate sharedApplication mocks that expose Crash in NSInvocation -dealloc when using OCMPartialMock erikdoe/ocmock#346 on iPhone 4 test runs.
  • The old build environment is still supported.
  • It will be removed when all pods have transitioned to test_specs and pod gen.
  • For now, see the instructions at https://github.com/firebase/firebase-ios-sdk/blob/master/Functions/README.md.
  • pod gen FirebaseAuth.podspec --local-sources=./ --auto-open

Copy link
Contributor

@renkelvin renkelvin left a comment

Choose a reason for hiding this comment

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

Mostly LGTM. Thanks Paul!

- travis_retry ./scripts/if_changed.sh ./scripts/pod_lib_lint.rb FirebaseAuth.podspec --platforms=ios
- travis_retry ./scripts/if_changed.sh ./scripts/pod_lib_lint.rb FirebaseAuth.podspec --platforms=tvos
- travis_retry ./scripts/if_changed.sh ./scripts/pod_lib_lint.rb FirebaseAuth.podspec --platforms=macos
# TODO (renkelvin) Uncomment next line when Auth backend is fixed to support the integration tests.
Copy link
Contributor

Choose a reason for hiding this comment

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

Got it.

@@ -161,11 +172,29 @@ jobs:
before_install:
- ./scripts/install_prereqs.sh
script:
# TODO investigate why macos tests hang for FirebaseAuth - keychain related?
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess so. The OS asks for password when you want to access to the keychain.

Copy link
Member Author

Choose a reason for hiding this comment

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

I suspect that it's related to the password, but somehow the dynamic framework version works.

didReceiveRemoteNotification:notification
fetchCompletionHandler:^(UIBackgroundFetchResult result) {}];

[self.mockNotificationManager verify];
}

- (void)testAppOpenURL_AuthPresenterCanHandleURL {
NSURL *url = [NSURL URLWithString:@"https://localhost"];
if (@available(iOS 9.0, *)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why no iOS 8.0?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a comment.

@paulb777 paulb777 merged commit daab8e3 into master Jun 6, 2019
@paulb777 paulb777 deleted the pb-auth-gen branch June 6, 2019 03:33
@firebase firebase locked and limited conversation to collaborators Oct 14, 2019
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.

None yet

3 participants