Split deviceAgnostic specific suffixes in options.

[Antondomashnev]

Hi there. First of all thanks for the great tool to test views.

Currently FBSnapshotTestCase has deviceAgnostic property that in case of true appends model name, os name and the screen size as suffix for the result filename. But for my purpose I need only device name + screen size, and my snapshots should not be different depending on the os name.
When Apple releases new sdk I need to update all my snapshots, that takes quite a lot of time just because they change the os version.

So this pr introduces the replacement for deviceAgnostic - agnosticnessOptions which is an NS_OPTION and the developer can combine different options to append to filename such as screen size, os version, model and localization.

It also provides the backward compatibility, deviceAgnostic becomes deprecated but still can be used.

What do you think about it?

[ghost]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks!

[ghost]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[Antondomashnev]

The Travis fails because of the cocoapods dependency. Shall I use git submodule as you do in facebook-ios-sdk ?

[orta]

You probably just need to rebase - master is green - ah, you did that, interesting

[Antondomashnev]

@orta yes, and the master does not have the cocoapods dependencies

[Antondomashnev]

Finally decided to go for git submodules and followed the structure from the facebook-ios-sdk

[Antondomashnev]

@nscoding Hey, is there a chance that this be merged?
Thanks =)

[dzenbot]

This is great, specially the ability for localisation segmentation. Why hasn't this been reviewed @nscoding?

It's actually a follow up from https://github.com/facebook/ios-snapshot-test-case/pull/158/files#r60323378 it seems.

[Antondomashnev]

@dzenbot I've been waiting for it since a while, I believe there should be a reason and it's not just stuck here =(

[nscoding]

thank you for working on this, I am not sure how I missed it. I promise to review this as soon as I can.

[mhollis1980]

Would this code handle the case where the test is testSomeThing() throws { }
The selector string looks like this testSomeThingAndReturnError: would it handle the ':' properly and change it to an '_' ?

[Antondomashnev]

@mhollis1980 hi, now it won't handle this case and it's the interesting case, I haven't seen this problem before, why do you want to change : to _ ?

[mhollis1980]

@Antondomashnev Moving from agnostic = true to agnostic = false all the file names would contain :. By adding this, moving between agnostic and not enables the files names to remain the same by applying the proper options. Then developers can decide how they would want to change the file names based on their needs.

[Antondomashnev]

@mhollis1980 when I submitted a PR all possible options were covered. However, if it's something new in the code this could be potentially not merged yet.

[mhollis1980]

@Antondomashnev. I don't want that at all. If you can merge please do.

[dzenbot]

This branch needs an update @Antondomashnev

[Antondomashnev]

@dzenbot thanks for noticing it. I'll do an update tonight 😄

[Antondomashnev]

@dzenbot I've rebased master into this branch, it's up to date now 👍

[soleares]

@Antondomashnev I believe FBSnapshotTestCase.podspec needs to be updated to include the new header file:

      "public_header_files": [
        "FBSnapshotTestCase/FBSnapshotTestCase.h",
        "FBSnapshotTestCase/FBSnapshotTestCaseAgnosticnessOption.h",
        "FBSnapshotTestCase/FBSnapshotTestCasePlatform.h",
        "FBSnapshotTestCase/FBSnapshotTestController.h"
      ],

Without it I get an error "Include of non-modular header inside framework module" after installing with CocoaPods.

[Antondomashnev]

@soleares unfortunately the PR is has not merged yet, so I can update the podspec in my local branch and you can point to it. Are you using FBSnapshotTestCase from my fork?

[soleares]

@Antondomashnev I merged your PR into my own fork and am using a modified podspec so it works great for me. I mentioned it because I assumed that the podspec change should be part of this PR. I'm not sure what the convention is though.

[Antondomashnev]

@soleares ah, ok, thanks. No sure if that PR is going to be merged at all, so before updating the podspec I'd like to wait for review from collaborators 😄

[alanzeino]

Hey there! We just moved the repo over here:

https://github.com/uber/ios-snapshot-test-case

I'm also PR'ing a longstanding change from our fork into the new repo which should hopefully meet this PR's requirements (though I haven't read all the code in this old PR):

uber/ios-snapshot-test-case#4

Let me know if we've missed anything!