Skip to content
This repository has been archived by the owner on Sep 20, 2023. It is now read-only.

FBSnapshotTestCase integration. Initial tests for AttributedStringView #688

Conversation

Austinate
Copy link
Contributor

@Austinate Austinate commented Oct 22, 2017

This PR adds (#656):

  • FBSnapshotTestCase integration:
    • Pod for Test scheme
    • Environment variables to main scheme: screen shot 2017-10-23 at 00 33 11
    • Added 'FailureDiffs/' folder signature to .gitignore (we would not want to have images from Failure Diffs in repository)
  • 2 simple tests for AttributedStringView (please correct me if there any additional tests needed since i'm not that familiar with codebase yet) :)

@BasThomas
Copy link
Collaborator

If we don’t add the snapshots to git, we can only test this locally... is that what we want?

Sent with GitHawk

@Austinate
Copy link
Contributor Author

@BasThomas actual snapshots are in repo. But we won't store failure diffs, since they should be picked up by CI and/or PR author for investigation :)

@Austinate
Copy link
Contributor Author

Austinate commented Oct 22, 2017

@BasThomas for example we have this in PR (and this should always be part of repo, ofc it doesn't make sense to store reference images locally only),
but if test will fails i will have this in Failure Diffs repo (and this part won't be in repo):
screen shot 2017-10-23 at 00 50 28
screen shot 2017-10-23 at 00 51 01

@BasThomas
Copy link
Collaborator

Ah, obviously. Nevermind me 😔

@Austinate
Copy link
Contributor Author

@BasThomas no worries :)

@BasThomas
Copy link
Collaborator

Thing is - we use them at work too. Just wasn't thinking. 😅

@Austinate
Copy link
Contributor Author

@rnystrom hi! I see some conflicts already, should i update PR or we're going to merge this later?

@Austinate
Copy link
Contributor Author

Hi, @rnystrom
Don't want to be annoying, but do we plan to have any actions on this? If so — i'll rebase, if not — probably would be nice to just close.
Thanks in advance!

@rnystrom
Copy link
Member

Hey! Sorry this fell off my radar. Yes! Let's rebase and I'll land.

@Austinate Austinate force-pushed the feature/initial_fbsnapshottestcase_integration branch from f058d8c to 47ae42a Compare November 22, 2017 12:47
@Austinate
Copy link
Contributor Author

@rnystrom Thanks for response! Rebased and ready to go

@Sherlouk
Copy link
Member

Haven't seen the source for this tweet, but sounds like FBSnapshotTestCase might not be maintained any more?

https://twitter.com/amlcurran/status/933002258790387713

@Austinate
Copy link
Contributor Author

@Sherlouk source is here
tl;dr: Unfortunately I recently archived the project as we no longer use this internally ... I am working on a solution though to provide a better owner for it. I'll keep you posted.

So i think there is a big chance that this still will be maintained

@rnystrom rnystrom merged commit d6cba1e into GitHawkApp:master Nov 22, 2017
@rnystrom
Copy link
Member

We use this at Instagram, it's a wonderful tool for building UI w/out having to spin up the entire app. Not to mention regression testing.

@Austinate Austinate deleted the feature/initial_fbsnapshottestcase_integration branch November 22, 2017 14:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants