-
Notifications
You must be signed in to change notification settings - Fork 72
feat: Decouple MParticle.m for Swift Migration #393
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
feat: Decouple MParticle.m for Swift Migration #393
Conversation
|
Could you detail how you tested this besides unit tests? |
- create test MPAttributionResult
- create test for MParticleSession
- create test for MParticleSession
- create test for MParticleOptions
ff27f56 to
0275754
Compare
Could you clarify what kind of additional testing you have in mind apart from unit tests? Since this change is purely a refactor without any business logic modifications, I focused on ensuring coverage with unit tests. |
|
Just some basic smoke tests. Get some events to show on the live stream in the cocoapods Example app and Higgs app. I understand that a lot of this is just cut and paste/reorganization but considering we know the unit tests are lacking we can't only rely on them and this is a fairly big change. |
I have checked with both the Example and Higgs apps, and confirmed that live events still show up as expected. |
BrandonStalnaker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for putting up with my requests 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, went over majority of this in person. Only comment is to maybe cancel the job that has been running for over 5 hours native-unit-tests (iOS, mParticle-Apple-SDK-NoLocation). It seems to be hung on BackendController Unit Tests with the last output being:
Test Case '-[MPBackendControllerTests testAutomaticSessionEnd]' started.
|
🎉 This PR is included in version 8.38.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
First stage of mParticle.m refactoring where some classes were moved from mParticle.m to their own files.
These classes were covered with tests. And obj-c tests were rewritten in swift.
Testing Plan
Reference Issue (For mParticle employees only. Ignore if you are an outside contributor)