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.

Sign up for GitHub

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

Jump to bottom

Enabling watchOS unit test #7480

Merged
merged 11 commits into from Feb 12, 2021
Merged

Enabling watchOS unit test #7480

merged 11 commits into from Feb 12, 2021

Conversation

charlotteliang
Copy link
Contributor

@charlotteliang charlotteliang commented Feb 8, 2021 •
edited

Enable watchOS on unit test

While enabling the test, also discovered and fixed two issues

  1. We are checking the wrong folder (Document folder) for storage size limit.
  2. The duplicate message check logic should be enabled for watchOS.

@google-cla google-cla bot added the cla: yes label Feb 8, 2021
paulb777
paulb777 reviewed Feb 8, 2021
View reviewed changes
Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

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

Do you mean a different Issue number? The one linked doesn't seem related?

FirebaseMessaging.podspec Outdated Show resolved Hide resolved
@charlotteliang charlotteliang changed the title Try enabling watchOS unit test [Draft] Try enabling watchOS unit test Feb 8, 2021
@google-oss-bot
Copy link

google-oss-bot commented Feb 8, 2021 •
edited

Coverage Report

Affected SDKs

  • FirebaseMessaging-iOS-FirebaseMessaging.framework

    SDK overall coverage changed from ? (80a4512) to 62.08% (873a3bd) by ?.

    Click to show coverage changes in 15 files.
    Filename Base (80a4512) Head (873a3bd) Diff
    FIRMessaging.m ? 68.63% ?
    FIRMessagingAnalytics.m ? 84.03% ?
    FIRMessagingContextManagerService.m ? 58.33% ?
    FIRMessagingExtensionHelper.m ? 82.68% ?
    FIRMessagingLogger.m ? 100.00% ?
    FIRMessagingPendingTopicsList.m ? 90.65% ?
    FIRMessagingPersistentSyncMessage.m ? 38.10% ?
    FIRMessagingPubSub.m ? 53.01% ?
    FIRMessagingRemoteNotificationsProxy.m ? 62.97% ?
    FIRMessagingRmqManager.m ? 58.95% ?
    FIRMessagingSyncMessageManager.m ? 76.92% ?
    FIRMessagingTopicOperation.m ? 0.00% ?
    FIRMessagingUtilities.m ? 44.23% ?
    NSDictionary+FIRMessaging.m ? 26.47% ?
    NSError+FIRMessaging.m ? 100.00% ?

Test Logs

@paulb777
Copy link
Member

paulb777 commented Feb 8, 2021

Thanks. I reopened the CocoaPods bug. It might be a relatively complicated fix for CocoaPods to add support for watchOS test_specs.

We might want to consider going straight to Swift PM for watchos unit tests. It currently gets some build errors:

Screen Shot 2021-02-08 at 3 37 36 PM

@paulb777
Copy link
Member

paulb777 commented Feb 9, 2021

Looks like the yml files are updated in the wrong branch ...

@charlotteliang charlotteliang changed the title [Draft] Try enabling watchOS unit test Enabling watchOS unit test Feb 9, 2021
@paulb777
Copy link
Member

paulb777 commented Feb 9, 2021

Can a matrix with watchos be added at https://github.com/firebase/firebase-ios-sdk/blob/master/.github/workflows/messaging.yml#L56?

@charlotteliang charlotteliang merged commit ff0ea54 into master Feb 12, 2021
@charlotteliang charlotteliang deleted the fm-watch-unit-test branch February 12, 2021 22:25
@firebase firebase locked and limited conversation to collaborators Mar 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@paulb777 paulb777 paulb777 approved these changes

@ryanwilson ryanwilson Awaiting requested review from ryanwilson

Assignees
No one assigned
Labels
api: messaging cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@charlotteliang @google-oss-bot @paulb777