Skip to content

Conversation

@plantpurecode
Copy link

We'd like to display some more debugging information about a test failure when it happens, so we've added a hook to support this.

Jacob Relkin added 3 commits June 23, 2014 14:26
… in a test run.

Added a SLTestCaseExceptionInfo object which encapsulates the relevant exception data.
* public/master:
  Support for using a prebuilt app instead of building one (fixes inkling#207)
  Incorporate PR feedback
  Allow SLAssert methods to be called from non SLTest subclass methods
  `-[UIView slAccessibilityIsVisible]` now reports an element's visibility on-screen, regardless of window. (fixes inkling#212)
  Make `SLPickerViewTest` 64-bit compatible.

Conflicts:
	Sources/Classes/SLTest.m
@MaxGabriel
Copy link
Contributor

This seems like a good idea to me. I wonder if Subliminal should also post NSNotifications for these kind of things. Subclassing SLTest means as soon as you want to add this kind of logging to an existing setup, you need to change the class of all your tests. Also, it means you couldn't make plug-and-play subliminal exception-logger library.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like @MaxGabriel's suggestion to post this information as an NSNotification. As Max says that'd be a more flexible arrangement, useful for this purpose and others.

@wearhere
Copy link
Contributor

Thanks for the PR @jacobrelkin! I think this is a great direction, particularly the refactoring of SLTest; I've just commented with a couple of ways we could make it cleaner.

Jacob Relkin added 2 commits August 22, 2014 18:36
* public/master: (35 commits)
  Add tests' superclasses' names to the default tag set. (fixes inkling#252)
  Pick up the latest OCMock for bug fixes.
  Add support for identifying alerts by message. (resolves inkling#246)
  Clarify `-[SLAlertTest testManuallyHandlingParticularAlertsInTestCode]` asserts.
  Update the CHANGELOG for 1.2 in progress.
  Revert "Ensure Travis uses the latest version of `xctool`."
  Refactor the description of the `SL_TAGS` environment variable for clarity.
  Tests and test cases may be conditionalized by tags based on an environment variable.
  Minor cleanup in `subliminal-test`.
  Add support for tagging tests and selecting tests based on tags.
  Refactor `SLDraggingTest` to use `+testCaseSupportsCurrentEnvironment:`.
  Add `+[SLTest supportsCurrentEnvironment]` and `+testCaseWithSelectorSupportsCurrentEnvironment:`.
  `+[SLTest supportsCurrentPlatform]` now returns `NO` if no test cases support the current platform.
  Move `-[SLTest run...]` to `SLTest+Internal.h`.
  Refactor `SLTest.h` to consolidate methods used for conditionalizing test runs.
  Put the UIA timeout around the entire UIA Alert handler
  Fix comment copy
  Add testCase alert handling integration test and dismissByTest
  Add a 2 second timeout to tapping the alert button
  Only parse a string as UTF8 once we have a full line to parse
  ...

Conflicts:
	Sources/Classes/SLTest.h
	Sources/Classes/SLTest.m
	Sources/Subliminal.h
@plantpurecode plantpurecode changed the title Added a hook on SLTest for when exceptions are thrown. Added a hook on SLTest exposing when exceptions occur within a test run. Aug 23, 2014
@plantpurecode plantpurecode changed the title Added a hook on SLTest exposing when exceptions occur within a test run. Added a hook on SLTest exposing exceptions that are thrown within a test run. Aug 23, 2014
@plantpurecode
Copy link
Author

Addressed all outstanding feedback on this pull request.

@MaxGabriel We decided that posting a notification should not be the concern of SLTest itself as the information contained within is bound to it; i.e. the test case selector. To make this data meaningful outside of SLTest you'd want to subclass SLTest and post a notification from within -testDidEncounterFailure:.

@plantpurecode plantpurecode changed the title Added a hook on SLTest exposing exceptions that are thrown within a test run. Added a hook on SLTest exposing failures that are encountered during a test run. Aug 25, 2014
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this file and the .m aren't used anymore.

@wearhere
Copy link
Contributor

Thanks for all this cleanup @jacobrelkin, removing all those local variables from -[SLTest runAndReport:...] is great. I've commented with some remaining functional issues.

I also talked with @justinseanmartin while you (quite understandably) were offline about using notifications after all. I see where you guys are coming from with the test case selectors being test data, but them also representing test cases is a fairly notable concept throughout the framework; and we thought of a couple of nice uses for notifications down the line, where method overrides could get messy. He can fill you in on more of the details.

As a heads-up, I've yet to make a stylistic pass, but I wanted to have you nail the functionality first. Thanks!

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.

3 participants