nui icon indicating copy to clipboard operation
nui copied to clipboard

Add Test Suite

Open tombenner opened this issue 12 years ago • 25 comments

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?

tombenner avatar Jan 08 '13 20:01 tombenner

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.

squarefrog avatar Mar 24 '14 21:03 squarefrog

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.

phatmann avatar Mar 25 '14 18:03 phatmann

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.

squarefrog avatar Mar 25 '14 20:03 squarefrog

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!

tombenner avatar Mar 26 '14 04:03 tombenner

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:

  • (-) Has been deprecated/Is on way to deprecation in Xcode 5.1 ?

XCTest:

  • (+) The standard test suite that comes with Xcode 5.
  • (+) Everyone has it
  • (-) Will still need OCMock or OCMockito to be useful here
  • (-) Slightly harder to read than the frameworks below. Each test needs to be named strictly - (void)testThatFooShouldBar

Kiwi

  • (+) Easy to read RSpec style bracketed syntax
  • (+) Has mocking and expectations built in
  • (+) Testing asynchronous/block code is fairly painless
  • (-) Requires 3rd party integration, fairly painless with Cocoapods
  • (-) Requires scalar values to be wrapped [[theValue(1) should] equal:theValue(1)];

Specta

  • (+) Easy to read RSpec style dotted syntax
  • (+) Wraps values for you expect(1).toEqual(1);
  • (-) Requires 3rd party integration, fairly painless with Cocoapods

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

squarefrog avatar Mar 26 '14 11:03 squarefrog

Any thoughts @tombenner @phatmann ?

squarefrog avatar Mar 28 '14 08:03 squarefrog

@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?

phatmann avatar Mar 28 '14 17:03 phatmann

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?

squarefrog avatar Mar 28 '14 20:03 squarefrog

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.

tombenner avatar Mar 30 '14 16:03 tombenner

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.

squarefrog avatar Mar 31 '14 14:03 squarefrog

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.

phatmann avatar Mar 31 '14 18:03 phatmann

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 avatar Mar 31 '14 18:03 squarefrog

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?

squarefrog avatar Apr 02 '14 09:04 squarefrog

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.

phatmann avatar Apr 03 '14 00:04 phatmann

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:

squarefrog avatar May 10 '14 21:05 squarefrog

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

phatmann avatar May 13 '14 18:05 phatmann

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 avatar May 14 '14 07:05 squarefrog

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!

squarefrog avatar May 29 '14 10:05 squarefrog

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!

phatmann avatar May 29 '14 18:05 phatmann

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 avatar May 29 '14 18:05 squarefrog

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:

squarefrog avatar May 29 '14 18:05 squarefrog

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.

phatmann avatar May 29 '14 18:05 phatmann

:+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:

squarefrog avatar May 29 '14 19:05 squarefrog

This is fantastic! Thanks so much, guys!

tombenner avatar May 31 '14 17:05 tombenner

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

  • [x] UIBarButtonItem
  • [ ] UIBarButtonItem back button, inherits from BarButton
  • [x] UIButton
  • [x] UIControl
  • [x] UILabel
  • [x] UINavigationBar
  • [x] UIProgressView
  • [x] UISearchBar
  • [ ] UISearchBar button, inherits from BarButton
  • [ ] UISearchBar scope bar, inherits from SegmentedControl
  • [x] UISegmentedControl
  • [x] UISlider
  • [x] UISwitch
  • [x] UITabBar
  • [ ] UITabBarItem
  • [ ] UITableView
  • [ ] UITableViewCell
  • [ ] The detail label of a UITableViewCell
  • [ ] UIToolbar
  • [ ] UITextField
  • [ ] UITextView
  • [ ] UIView
  • [ ] UIWindow

squarefrog avatar Jun 02 '14 14:06 squarefrog