feat: SDKE-64 Improve mParticle.m test coverage in swift 2

[denischilik]

Summary

This is the second PR in the series aimed at improving the test coverage of mParticle.m. The first PR is #406

Details

• Began migrating Objective-C code into Swift. The migration process followed these steps:
  1. Created all required tests for the target method.
  2. Verified that the tests worked as expected.
  3. Migrated the implementation into Swift.
  4. Re-ran and validated all tests.
• Added a few setters to allow initializing the mParticle class with mocks for testing purposes.
• Exposed some classes via the umbrella header to ensure they are accessible from Swift code.

Testing Plan

• Was this tested locally? If not, explain why.
• {explain how this has been tested, and what, if any, additional testing should be done}

Reference Issue (For mParticle employees only. Ignore if you are an outside contributor)

• Related to https://rokt.atlassian.net/browse/SDKE-64

[nickolas-dimitrakas]

LGTM

[BrandonStalnaker]

So I think I can see the issues that are forcing you to make some of these decisions. I'm just worried that this PR is in essence starting the refactor, with the risks that has, but it's only doing it halfway which forces you to make a bunch of compromises.

I thought the point of this work was to increase find ways to increase testing before we started the refactor. Is that right @rmi22186 ?

[denischilik]

So I think I can see the issues that are forcing you to make some of these decisions. I'm just worried that this PR is in essence starting the refactor, with the risks that has, but it's only doing it halfway which forces you to make a bunch of compromises.

I thought the point of this work was to increase find ways to increase testing before we started the refactor. Is that right @rmi22186 ?

I see your point — thanks for raising it. The reason I approached it this way is that after making the code testable and covering those parts with tests, I realized we could already migrate them to Swift safely. My goal here was to demonstrate one possible approach: add test coverage first and then immediately migrate the code, so we don’t have to revisit the same parts later. I thought this would be a good opportunity to share and discuss this approach with the team.

If we decide not to convert to Swift right away, I’m happy to keep just the tests without any code changes.

[rmi22186]

@denischilik - yup I was thinking about what @BrandonStalnaker was raising here. Let's keep it to test coverage only. We had discussed creating some sort of E2E framework with a simulator app or something that fires off X events, X identity calls, and we have a test suite to ensure any network requests adhere to an expected payload. That way, we are more confident about the payload never changing and any behavior relative to the swift migration.

[denischilik]

@denischilik - yup I was thinking about what @BrandonStalnaker was raising here. Let's keep it to test coverage only. We had discussed creating some sort of E2E framework with a simulator app or something that fires off X events, X identity calls, and we have a test suite to ensure any network requests adhere to an expected payload. That way, we are more confident about the payload never changing and any behavior relative to the swift migration.

Removed changes related to swift migration CC: @rmi22186 @BrandonStalnaker @nickolas-dimitrakas

[nickolas-dimitrakas]

LGTM

[BrandonStalnaker]

By and large this looks good. I have a suggestion for a test that may or may not work but I'll leave whether it goes in up to you.

Edit: Oh! but please make sure all tests have past successfully on the PR before merging.

[mparticle-automation]

🎉 This PR is included in version 8.39.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀