Skip to content

Conversation

@wmora
Copy link
Contributor

@wmora wmora commented Jan 10, 2019

When running the tests in record mode, they fail with the message: Snapshot comparison failed: (null) because it was checking for the test result before checking if the test ran in recordMode. This PR changes the validation order.

@CLAassistant
Copy link

CLAassistant commented Jan 10, 2019

CLA assistant check
All committers have signed the CLA.

@reidmain
Copy link

Thanks for this @wmora. I'm 100% onboard with this change. But if we make it couldn't we move this return statement up into the other if (self.recordMode) { statement? If we are in record mode we won't actually be doing anything to change the testSuccess variable so we can just always return early.

@reidmain
Copy link

I am trying to get a release out this week so I'll go ahead and accept this PR and then put another one up to move the code around.

@reidmain reidmain merged commit a674dde into uber:master Jun 27, 2019
lukaskukacka pushed a commit to lukaskukacka/ios-snapshot-test-case that referenced this pull request Jan 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants