tombenner / nui

Style iOS apps with a stylesheet, similar to CSS
MIT License
3.76k stars 459 forks source link

Add Test Suite #48

Open tombenner opened 11 years ago

tombenner commented 11 years ago

Because NUI uses custom graphics (e.g. gradients) that cannot be tested by checking elements' properties programmatically, one approach for this might be to add elements to a view, render that view to an image, and then compare that image with an authoritative image of how the view should be rendered.

If each property of each element was tested incrementally, though, we'd need to work out how to organize those across authoritative images, as having a different authoritative image for every combination in that Cartesian product (element x property) might not be realistic.

Other thoughts on how to approach this?

squarefrog commented 10 years ago

There are a couple of ways you could test this.

Something like https://github.com/kif-framework/KIF would allow you to generate screen shots and compare them. This would be tricky as you could get failing tests switching iOS 6 to 7.

For many of the properties standard XCTest/OCUnit tests would work fine. Even better when you add in OCMock to really separate out dependancies.

Something like Kiwi/Specta could help with gradient testing. You can set expectations like:

[[foo should] receive:@selector(bar:) withArguments: //gradientCode, nil];

I'd probably have to dig in to the code to see how you setup the gradients.

In any case i definitely think getting a test suite and using something like Travis could definitely improve confidence when merging pull requests.

phatmann commented 10 years ago

I am using KIF and OCMock on my current project so I am willing to jump in on this one. @squarefrog are you also interested in helping out and writing some tests once we get the basic framework set up?

KIF does not provide screen comparisons, but I have been strongly contemplating adding this, since I used to use Zucchini and really liked this functionality. Without screen compares, any NUI test would not be complete.

squarefrog commented 10 years ago

Yes definitely. I'd like to contribute more to Open Source projects. I usually use Kiwi, but I'm happy with XC/OC with OCMock. I've not used KIF but this would be a great opportunity to learn it.

Ideally I'd like to pair for a few tests, as I've not done a lot of view specific tests. This could just be as simple as getting a :+1: on a PR.

tombenner commented 10 years ago

The absence of tests is easily NUI's biggest delinquency, so getting the ball rolling on this would be hugely, hugely appreciated. I haven't done a thorough survey of Obj-C testing frameworks recently, but am absolutely looking forward to hearing what path you guys feel is best. Many thanks!

squarefrog commented 10 years ago

I suppose we should select the framework based on what we need to achieve. For general property setting tests we just need a framework that allows expectations. OCMock, Kiwi and Specta all support this. So we should be ok. Lets look at pros and cons of the testing frameworks:

OCUnit:

XCTest:

Kiwi

Specta

Kiwi is what I use day to day, so I'm definitely more familiar with it than other frameworks.

squarefrog commented 10 years ago

Any thoughts @tombenner @phatmann ?

phatmann commented 10 years ago

@squarefrog, thanks for the great list!

Note that I have direct experience with OCMock, KIF, and XCTest, but none with Kiwi or Specta.

Before we make a decision on the framework, let's decide how we want to test. In order to guarantee that NUI is working is correctly, we have to look at the pixels. Just checking a control's properties is not sufficient. For example, how can you make sure a shadow or gradient is being drawn correctly? It is true that some NUI styles can be checked via properties (e.g. fonts), but it seems simpler and more consistent to use pixel comparison for all tests.

We would programmatically generate a screenful of controls with various stylings applied to them, take a screenshot, and compare to the reference screenshot. We would need reference screenshots for all combinations of iOS 6/7 and iPad/iPhone.

We can use this library for screenshot generation and comparison, driving it from XCTest. We would not need OCMock, and I don't see any benefit to using Kiwi or Specta for such simple tests.

We don't need to tap any elements, since we can use the highlighted property instead. So KIF won't be needed.

Thoughts?

squarefrog commented 10 years ago

I can't argue with any of those points. The fewer external dependancies we need, the better.

I presume the reference images will need recreating each time a significant new feature or property is added?

I'll do some research about that test library this weekend, but on a cursory glance it definitely seems the quickest way to get strong coverage going on.

Facebook seems to be releasing some really excellent tools recently!

I do definitely feel classes like the .nss parser could be a good candidate for some delicious XCTest logic tests. Is there any specific error handling/linting currently?

tombenner commented 10 years ago

FBSnapshotTestCase looks promising to me, too. Completely agreed re: keeping a minimal number of dependencies. Since screenshot-based tests are by far the most useful tests for us, perhaps we could begin by writing FBSnapshotTestCase tests for a handful of properties across different elements (in varying usage contexts, too) and see how that fares. As you say, @phatmann, we'll want reference screenshots for all combinations of iOS 6/7 and iPad/iPhone.

@squarefrog, definitely let us know what you think regarding FBSnapshotTestCase vs. other frameworks. I'm sure we'll find a number of cases that it wouldn't support (like the NSS parser). Perhaps we can focus on the screenshot tests initially and then that'll help us discover what those other cases are and inform our choice of a test framework to handle them. Completely open to other paths forward, though.

squarefrog commented 10 years ago

I didn't get chance to check it out this weekend, but I say lets just go for it. My only initial concern is how much of a pain will it be to maintain the tests? I presume new appearance selectors only really appear during iOS updates, so potentially the reference images will need recreating?

Although I suppose it's only really going to affect the control that Apple updates.

Well lets get the ball rolling with some snapshot tests like you say, and at this point i'm fairly confident that the future logic tests can be covered with plain old XCTest.

phatmann commented 10 years ago

We would be using XCUnit as our framework. FBSnapshotTestCase extends that framework to provide screenshotting and image comparison. So we would use XCUnit for the NSS Parser unit tests, and then use FBSnapshotTestCase on top of that to check the styling.

I say we can commit to using XCUnit right now. One of us needs to play with FBSnapshotTestCase a little before we commit to it. Luckily I want to evaluate it for my current project anyway, so I will give it a run and let you know, probably today or tomorrow.

squarefrog commented 10 years ago

Most good frameworks inherit from XCTest which is nice as it allows you to re-run single tests or classes in Xcode 5.

Ok sounds great! I'll try take a look as well so I'm at least on the same page as you guys.

squarefrog commented 10 years ago

I've just gone to add a unit test target to my fork of NUI and I suddenly remembered XCTest is iOS 7 only.

I guess we shall have to have two targets: NUIOCUnitTests and NUIXCTests. Any better solutions?

phatmann commented 10 years ago

To keep things simple, let's focus on iOS 7 and get the tests up and running using XCTest. We can worry about iOS 6 later.

squarefrog commented 10 years ago

I've started work on this today. It's pretty interesting to discover the challenges of such a sledgehammer approach of testing! On a side note, I've already discovered the first bug in nui through tests :+1:

phatmann commented 10 years ago

This is great to hear! I look forward to finally seeing a test suite emerge.

squarefrog commented 10 years ago

I want to get a single class tested and my fork pushed before I continue on with other classes so everyone has an opportunity to say whether the test flow is appropriate or not.

As it stands, I'm creating a separate test only stylesheet, which looks hideous, but should be very easy to spot any issues. The individual tests are a way to exercise as many properties as possible using a single image. Therefore, there has to be a set of images for flat background color, and a set for gradients.

Ideally, the test images should be created using standard appearance code and then verified against NUI, but this is completely unmanageable and fragile so we will have to do a lot of manual visual testing until we know the reference images are correct.

squarefrog commented 10 years ago

I've spent the morning writing tests, and I think we can actually exercise a lot of the properties without images. Have a look at my tests branch and see what you think.

I think the only properties that should be tested with FBSnapshotTestCase are the ones that would be too difficult to test with comparisons. For example, the background image tests which require a generated image.

The advantage with exercising using asserts instead of images is that if a property changes or fails, we know immediately where the error is happening. So this would mean writing a test per property.

If we did the same using images we would end up with many images which would pollute the project file and make it much larger to clone or try.

Have a look through and tell me your thoughts, I'll stop working on it now until you've both had the opportunity to look through.

Cheers!

phatmann commented 10 years ago

Hey, these tests look very cool and I am excited to run them. However, I could not get them running on your test branch. The first problem was that some of the Pod files were missing. Running pod update seemed to fix that. But then when I ran the tests I got an error saying:

 Library not loaded: /Developer/Library/Frameworks/XCTest.framework/XCTest

I noticed that there were two XCTest frameworks in the project. I removed both, added one back, but the problem persisted.

To reproduce these issues, I recommend you do a fresh checkout of your branch and try running the tests with it.

Hopefully you can resolve these issues quickly and then I can see your awesome tests!

squarefrog commented 10 years ago

Make sure you run the iOS 7 simulator as XCTest does not load in anything less.

I found this issue when setting up Travis for another project.

The reason the Pod files were missing is that I haven't checked them into source control, as I don't feel this is appropriate. Ideal we will run these tests on Travis, which can be configured to run pod install.

I'll look into the duplicated frameworks - thanks for the shout!

squarefrog commented 10 years ago

Again, this is very much a work in progress fork, so any changes are most welcome, I'll probably squash or rebase before submitting a Pull Request so don't worry if they see insignificant :rocket:

phatmann commented 10 years ago

I got the tests working in iOS 7. Fantastic work! Let's merge them into master ASAP.

I recommend that we check in all the Pod files. We don't want users to have to run pod install when evaluating NUI. And running unit tests is a part of that.

squarefrog commented 10 years ago

:+1: as you wish! I'll check them in soon.

I don't want to submit a pull request just yet, I'd like to get the button rendering at least fully tested. If you look at the test class there are a few things missing.

I might take a look at the issues I've opened as a matter of urgency though, as I don't really fancy pulling in a bunch of failing tests.. At least not while my name is attached to them :laughing:

tombenner commented 10 years ago

This is fantastic! Thanks so much, guys!

squarefrog commented 10 years ago

Just going to leave this here as a way for me to keep track of what has been done.