Adding OCMock as gitmodule for building tests.

[noobs2ninjas]

For some reason we've never added OCMock as an official dependency in Carthage. It's just automatically loaded because of the Facebook SDK till now.

[noobs2ninjas]

@drdaz it would be cool if I could get your approval so I can work on a new issue. I think we will need to update the podspec to pull Bolts from our fork. Not sure how/if we can do that in the podspec. Documentation doesnt mention it although its very possible on podfiles.

[drdaz]

Hey there, those iOS tests look funny. I'm not sure I've seen those first 2 errors here before. Can we confirm that these are some of the 'expected' errors for the iOS tests?

I'm not sure about the podspec, since I haven't tried this manoeuvre before, but maybe it's a case of changing the naming in our Bolts podspec, and then matching that in our SDK podspec?

[noobs2ninjas]

@TomWFox think i could get you to approve so I can get this done?

[TomWFox]

@noobs2ninjas sorry for the delay, I've had a look. It looks like those iOS test failures are relating to OCMock, has the version of OCMock we're using changed?

[noobs2ninjas]

@TomWFox Thanks for getting back. Heres the error for each one.

Twitter:
Failing tests:
ParseTwitterUtils-Tests:
-[OAuth1FlowDialogTests testRotation]
** TEST FAILED **

macOS:
Failing tests:
ParseUnitTests-macOS:
-[KeyValueCacheTests testConstructors]
-[PinningObjectStoreTests testFetchPin]
-[FileControllerTests testDownloadStreamSharesOperations]
** TEST FAILED **

iOS:
Failing tests:
ParseUnitTests-iOS:
-[FieldOperationDecoderTests testDecodingAddRelationOperations]
-[KeyValueCacheTests testConstructors]
-[PinningObjectStoreTests testFetchPin]
-[PurchaseControllerTests testBuyProductsAsync]
** TEST FAILED **

As you can see these errors are from failed tests. Which means that the project and OCMock did build successfully or the tests never would have begun. Not sure where you saw OCMock issues, maybe you ended up on a past build page. These however are building just fine. Now I just need to merge this so we can work on fixing the failing tests.

[TomWFox]

[noobs2ninjas]

@TomWFox Im guessing it has. OCMock used to be a set version that was a dependency of the Facebook SDK. Facebook has since changed that so I just took whatever version was current when I added it as a git module. My guess is that they switched some of their initializers for OCMStub. Either way though. I think this would fall under the category of fixing the tests which is on my TODO list but a bit beyond the scope of this specific pull request. My only goal was to add OCMock via git module and make sure every test was building. In this case the target did build, its only after the tests have started do we get an error.

So, can we get this one approved so I can move on to a new PR to fix the tests among other things?

[drdaz]

Getting the tests back to the state they were in before all the build environment changes is precisely the point of this PR. The new errors look likely to be a direct result of the changes made to the environment.

I won't approve a PR that adds new test errors to the mix. If the tests can be fixed (or rather the new errors can be eliminated), they can and should be fixed here imho.

[TomWFox]

Ok, I’m happy with that as long as we at least attempt to fix these before release.

[drdaz]

lol 🤷🏽‍♂️

incorect approval

[TomWFox]

Tbh @drdaz is right on this one, I shouldn’t have approved, I just want to get things moving but really fixing the tests isn’t outside of the scope of this pr.

Is there any practical reason why the tests can’t be fixed in this pr?

[TomWFox]

So it looks like we were running OCMock v3.4.3 (link) before it was removed.

If there isn't a specific reason to be on a different/newer version, we could try reverting to that version as a quick fix, unless you want to try and fix the tests as it is?

[noobs2ninjas]

@TomWFox unfortunately one of the downsides to using gitmodules is it doesn't allow for designating a release. However, these failed tests have been failing well before the last version. They where failing before I even started changing thing. They failed when facebook was the one who included OCMock. The only thing that OCMock has changed is rather than just failing the test, they now create their own exception with a more detailed explanation. In fact, when 3.0 came out YEARS ago, they made changes that would have required you to convert all your objects to OCMockObjects which is the error we got. So, it seems they are just making their errors more visible. Even after fixing that error I had 3 more tests fail.

Anyway, earlier I had had commented and complained about some of these being here for so long and how it wasn't my update that even broke them. Well, I got bored and started fixing things. So heres where we stand now. I fixed testConstructors, testFindObjectsCacheOnlyCorruptJSON, testDownloadStreamSharesOperations, and testBuyProductsAsync. So, all test but testFetchPin, and testFetchPinCaching.

The odd thing about these tests is that they BOTH succeed sometimes and they always succeed if you run them individually. Other times one fails and the other doesnt. Sometimes I get 'The class PFPin must be registered with registerSubclass before using Parse.' or unrecognized selector. Other times it straight up crashes with no exception. What I suspect is that other tests can change the environment and setup these tests to fail. Which would make sense as to why they succeed when ran individually. It would also explain the randomness because these tests aren't necessarily ran in order. Also, it may not even be the tests themselves as we have issues about offline pinning.

So, I was hoping that the tests I've fixed for now will suffice as these two tests definitely have some underlying issues than simply the individual tests themselves. I worked on them for an hour and still got no better results. In fact both tests passed. Then failed on Circle and then failed afterwards again on my computer and I have no clue why.

@drdaz not sure how well you know the testing setup. I have a suspicion we have a offline storage issue and also if you have any knowledge of the unit testing. I see classes like PFStrictProtocolMock and PFStrictClass mock etc.. but OCMock for a while now requires using OCObjectMock and OCProtocol mock. So it seems we are mocking things twice. Anyway, thats a whole issue in itself is cleaning everything up to make things more efficient and cleaner. Just was hoping you might be able to save me some time and also help me dig down to the bottom of the pin issues.

Anyway, hopefully we can get this merged so I can move on to those deeper issues.

[drdaz]

If you've fixed those janky tests, that would be awesome!

However, these failed tests have been failing well before the last version. They where failing before I even started changing thing.

Link?

@drdaz not sure how well you know the testing setup. I have a suspicion we have a offline storage issue and also if you have any knowledge of the unit testing.

I haven't dug into how offline storage works really. Some years ago it burned me badly as a user of the SDK. I haven't felt like messing with it since.

[drdaz]

Wait, looks like the OCMock failures were already fixed? This looks good!👌🏼

I committed blindly. If it breaks anything, we roll it back.

Any idea about these warnings?

Multiple targets match implicit dependency for product reference 'OCMock.framework'. Consider adding an explicit dependency on the intended target to resolve this ambiguity. (in target 'ParseUnitTests-iOS' from project 'Parse')
Capabilities for Signing & Capabilities may not function correctly because its entitlements use a placeholder team ID. To resolve this, select a development team in the ParseUnitTests-iOS-host editor. (in target 'ParseUnitTests-iOS-host' from project 'Parse')

It's in multiple projects. We've seen this one cause build failures before.

[noobs2ninjas]

@drdaz Yup! I did a bunch of complaining. Then i deleted those comments and decided to go ahead and fix it.

It's in multiple projects. We've seen this one cause build failures before.

Yea its weird. I ran the build dozens of times and it never failed. It should be easy to fix a lot of the warnings. They all have to do with linking dynamic frameworks. We have lots of warnings based around the same issue. To get those cleaned up I'll have to remove all dependencies from every target and dynamic target and relink everything in a uniform way. It's not hard. Just a bit time consuming and tedious. For now I'm not worried about us having any issues with it.

Edit: Also yes. If it breaks we roll it back.

[noobs2ninjas]

Just waiting approval and we should be good.

[drdaz]

Great work! 🙏🏼👌🏼