Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow passing in pre-ordered arrays of tests to SLTestController #94

Closed
wants to merge 10 commits into from

Conversation

j-mutter
Copy link
Contributor

I'm not sure if you guys are interested in this, or a different implementation of the same thing, but we need the ability to run some tests in a specific order before others, so we're handling the randomization before feeding them to the SLTestController.

@wearhere
Copy link
Contributor

This is a really nice implementation of pre-orderings, @j-mutter --cleanly implemented, well-tested and documented. I think I'm going to have to turn you down though. :) I gave a bit of reasoning in discussion of a similar issue--see this comment and my response. I'd be curious to hear your use case though and if you think you have a solution to my worries that orderings would make the tests brittle.

@j-mutter
Copy link
Contributor Author

Yeah we're in a similar situation, however it's not simply a matter of being logged in or not. We do a fairly hefty import process on sign in, so modifying the app to allow us to switch this state(unload and reload the database, preferences, image cache, etc) is not ideal, and doing a full import for every test is out of the question.

@wearhere
Copy link
Contributor

If the import needs to be done once and only once, what about dispatch_once'ing it within the -setUpTest of an abstract test? You could even "test" that process, i.e. put asserts in there if you wanted (if the success of that process gated the rest of your tests).

@j-mutter
Copy link
Contributor Author

That could work, but we also have a few pre-import things we want to test (auth failures, etc). I'm open to suggestions, but we really need to be able to run a handful of things in a specific order and this was how we opted to do it.

@wearhere
Copy link
Contributor

If I understand your situation correctly, you guys have two groups of tests: pre- and post- import. It's not clear to me that the order within those groups matters so much as that all the pre-import tests run before any of the post-import tests.

So, what would you think about a mechanism that let you order, not individual tests, but groups of tests? For instance, make multiple calls to a new, synchronous version of runTests:

// at the end of `applicationDidFinishLaunching:`
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0), ^{
    NSMutableSet *allTests = [[SLTest allTests] mutableCopy];

    SLLog(@"Running pre-import tests…");

    // could be other tests too
    NSSet *preImportTests = [NSSet setWithObject:[SLTest testNamed:@"PreImportTest"]];
    [[SLTestController sharedTestController] runTestsSync:preImportTests];

    SLLog(@"Running post-import tests…");

    [allTests minusSet:preImportTests];
    [[SLTestController sharedTestController] runTestsSync:allTests];
});

In this scenario, the tests executed in the second run would all subclass an abstract test AuthTest, which, in its implementation of -setUpTest, dispatch_once'd the import process. That way, you'd only have to do the import process once; and you could guarantee that it would happen at the crucial divide between your pre- and post-import tests; but within those groups, you wouldn't sacrifice randomness.

@j-mutter
Copy link
Contributor Author

That's essentially what we're doing - we have a pre-import class with all the auth tests and stuff, then we have a class that only does the import, then we have the rest of the test classes. The tests in each group are still run randomly.

The reason we added the ordering is so that we could queue up the pre-import, then the import tests followed by the rest - I had tried chaining runTests: before implementing this, but ran into issues which I now forget. This was just the path of least resistance.

I would prefer to do something like you've suggested. It's definitely much cleaner. runTestsSync doesn't seem to be implemented - is that something you're working on currently, or should I throw it together?

@j-mutter
Copy link
Contributor Author

Closing this - we implemented runTestsSync with an abstract 'loggedInTest' base class. I'll get it cleaned up and open a new PR

@j-mutter j-mutter closed this Feb 10, 2014
@wearhere
Copy link
Contributor

Oh slick! I look forward to seeing that.

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.

None yet

3 participants