-
Notifications
You must be signed in to change notification settings - Fork 54
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 SLAssert methods to be called from non SLTest subclass methods #217
Allow SLAssert methods to be called from non SLTest subclass methods #217
Conversation
|
||
// Only execute the test case if set-up succeeded. | ||
if (!caseFailed) { | ||
for (NSUInteger i = 0; i < 1000; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A potential concern with letting the assertions be called outside of a test would be that they could be called outside of the current test's exception-handling infrastructure. Would you just leave it up to the user to be aware of/guard against that themselves? Two alternatives in which you might be interested would be to:
|
Yea, I'd leave it up to the user to be aware of only throwing these exceptions inside of test code. Let me look at our architecture and see if I can refactor to something else that makes sense. |
} | ||
@implementation SLTest | ||
|
||
static NSString *_lastKnownFilename; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please prefix static variables with two underscores to differentiate them from instance variables.
I don't think my objections above are that strong. If you can address my stylistic comment above I'll merge this then. |
Done. I also added a comment to SLTest.h on SLAssertTrue for how to do this. I think there is already a bug saying that we should expand on test case organization, maybe we could add on our use case to that ticket. The current design is to break out a class for each screen of our UI with helper methods. This will allow containing of logic for interacting with a particular screen to a single place. |
should only be used from test setup, teardown, or execution methods as well as those | ||
methods called from within. | ||
|
||
@warning If you perform operations on the main thread within your test case, you can't |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good point, though I find the phrasing a bit confusing. What do you think about
@warning If you use an assertion inside a dispatch block--if your test case dispatches to the main queue, for instance--you must wrap the assertion in a try-catch block and re-throw the exception it generates (if any) outside the dispatch block. Otherwise, the tests will abort.
Some small comments on documentation. I agree that a discussion of test case organization is out of scope for this request/more appropriate for the wiki. |
Adding a comment to SLTest.h describing when SLAssert should be used Renaming the static variables to use double underscores
Incorporated the last 2 pieces of feedback. It is unfortunate that the SLAssert documentation doesn't seem to appear in the published documentation anywhere. It seems that is a limitation of appledoc in general though, so not much you can do about it I guess. |
Yeah, I don't think Looks good! I'll merge as soon as Travis finishes up. |
…rom-non-sltest-subclasses Allow SLAssert methods to be called from non SLTest subclass methods
👌 |
What do you think about this? I was planning on building a library of helper methods to build up the test cases, but unless they subclass SLTest, I can't call the assert methods. This doesn't seem right to me.
This is my proposed solutions, but I wanted to get some thoughts.