[google_sign_in] Remove use of OCMock

[stuartmorgan-g]

Wraps Google Sign In SDK classes with protocols, so that we can inject fakes that implement the protocols instead of using OCMock to mock the actual classes. This will allow conversion of the tests to Swift, in preparation for migrating the entire plugin to Swift.

For now the protocols are implemented in production with passthrough wrappers, so this adds a fair amount of boilerplate code to the plugin, but almost all of it will be removed when the plugin implementation is converted to Swift since we will be able to use extensions to make the SDK classes conform to the protocol instead of maintaining wrappers.

Unblocks flutter/flutter#119103

Pre-Review Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene page, which explains my responsibilities.
• I read and followed the relevant style guides and ran the auto-formatter.
• I signed the CLA.
• The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
• I linked to at least one issue that this PR fixes in the description above.
• I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under¹.
• I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under¹.
• I updated/added any relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or I have commented below to indicate which test exemption this PR falls under¹.
• All existing and new tests are passing.

Footnotes

1. Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩² ↩³

[gemini-code-assist]

Code Review

This pull request is a significant and well-executed refactoring to remove the OCMock dependency from the iOS Google Sign-In plugin tests. By introducing a protocol-based abstraction layer for the Google Sign-In SDK, you've greatly improved the testability of the native code, which is a crucial step for the planned migration to Swift. The use of wrapper classes for production and hand-crafted fakes for testing is a clean and effective pattern. The changes are extensive but logical and consistent throughout the codebase. I have a few suggestions to further improve the code's robustness and adherence to Objective-C best practices.

[gemini-code-assist]

Properties for objects that have mutable counterparts (like NSString, NSArray, NSDictionary, NSURL, NSDate) should be declared with copy instead of strong. This prevents the internal state of the object from being changed unexpectedly by another object that holds a reference to a mutable version. This is a standard Objective-C best practice.

This applies to email, name, and imageURL here, and also to properties in other test fake classes:

• TestToken: tokenString, expirationDate
• TestSignInResult: serverAuthCode
• TestGoogleUser: userID, grantedScopes, requestedScopes

Suggested change

	@property(strong, nonatomic) NSString *email;
	@property(strong, nonatomic) NSString *name;
	/// A URL to return from imageURLWithDimension:.
	@property(strong, nonatomic) NSURL *imageURL;
	@property(copy, nonatomic) NSString *email;
	@property(copy, nonatomic) NSString *name;
	/// A URL to return from imageURLWithDimension:.
	@property(copy, nonatomic) NSURL *imageURL;

[stuartmorgan-g]

Unfortunately the compiler gets angry if you re-declare a public readonly property as copy, even if it's specifically being changed to readwrite. That seems like a bug, but we're stuck with it.

I don't want to make the protocols use copy because they are duplicated exactly from the SDK, in anticipation of the switch to Swift class extensions to add protocol conformance.

(I could instead make these ivars, add manual implementations of the read-only properties, and declare an initWith... to set them all, but that's a bunch more test code to fix something that should not actually matter in tests.)

[stuartmorgan-g]

The reason this uses GIDConfiguration directly, instead of introducing an FSIGIDConfiguration protocol like everything else, is that for whatever reason the SDK exposes this as a data class with public initializers, unlike the other classes that look data-class-y (GIDSignInResult, GIDToken, etc.) which don't have public initializers.

Since we can make instances of this class with arbitrary data in tests, we don't need to wrap it to allow for that.

[stuartmorgan-g]

I went ahead and moved this code into the view provider abstraction, since that's where the non-deprecated replacement codepath will live, it's what I have already done in at least one other plugin where I introduced a view provider, and it means we could write tests for code that calls this without hitting issues with trying to use the actual UIApplication in a unit test.

[stuartmorgan-g]

I have no idea why this line was here; it was making an instance without any test DI that we were never using in tests. That's generally not something we want to do in tests, so I removed it while I was adjusting the DI around it.

[stuartmorgan-g]

This is the only place where an existing test was changed in a way that wasn't just adapting the OCMock logic to the new approach. That's because this looked like an actual test, but it wasn't: the OCMExpect call was never actually triggered, so none of the XCTAsserts were being executed. Reading from the Info.plist is logic that is in the SDK, not in the plugin. That's why line 186 was making a different plugin instance that didn't inject a mock Google Sign In instance... but that also meant that an expectation on self.mockSignIn never fired.

So basically this test was just testing that the SDK didn't crash, not that it actually had the expected behavior. I considered fixing this to read the values from the actual config, but having a plugin unit test that is asserting SDK behavior rather than plugin behavior is weird, so I changed it to instead assert the plugin behavior we want, which is to not set a config so that the SDK uses the one it creates by default from the Info.plist.

[stuartmorgan-g]

Mostly I didn't do test backfill in this PR, but I noticed we had a lot of tests of error conditions for addScopes:... but nothing that tested that we actually requested the right scopes, so added that quickly while I was here.