[WIP] Reporting #233

paulyoung · 2015-01-08T08:41:42Z

This represents some progress towards #9. I've actually experimented a little bit and taken things a step further but think my findings warrant some discussion first. There's also some good discussion/ideas in kiwi-bdd/Kiwi#449 which I intend on introducing here in the near future.

I've spent a great deal of time investigating how XCTest currently works and does test reporting thanks to modocache/XCTest-Headers.

Initially I tried removing XCTest's own test observers using various approaches but I couldn't prevent the output from being logged. When I could, it broke the integration with Xcode. Hence, I've essentially re-implemented the appropriate methods to only integrate with Xcode. This leaves room for our own output in whatever format is desired.

When I say "re-implemented", I originally did this via categories but now via method swizzling. This should provide support for us to not do anything should the default reporter be desired.

This concludes the current progress. Discussion on how to proceed below.

I explored writing my own test observation center which hooks into XCTestObservationCenter. Like the original, observers can hook into the observation center in order to be notified of progress and print their output to the command line. So far I have the following reporters:

one with output practically identical to the default we all know and love
one that prints dots for passing examples, F for failures
a broken BDD reporter (see below)

These are fairly easy to implement by creating a Swift class that inherits from NSObject and conforms to a TestObserver protocol that I defined. So far I've been using fputs("...\n", __stderrp) to log output, but a particular implementation could do something entirely different.

This all works really well and could even be a separate project from Quick/Quick since it doesn't depend on anything other than XCTest. However, there are some caveats:

suite and test names are provided by XCTest and therefore require a lot of massaging in order to parse out what we consider to be the string passed to describe, context, it. I've done a lot of the work to extract the correct strings but punctuation and other special characters are lost forever. It may be possible to store these strings somewhere and access them at the right time, either via method call or dictionary lookup.
getting the nesting right when printing is proving really difficult. It seems doable but there's a lot of bookkeeping involved.

I think it might be worth exploring this a little bit more, but we may prefer to hook into Quick and have it report directly in the BDD language we expect.

* To support reporter configuration.

paulyoung · 2015-01-08T08:44:27Z

Totally forgot to mention that this currently breaks running the tests for the Quick project itself. I know we have some special handling there and I've yet to investigate further.

I've been testing this using a demo project that runs its tests using my local version of Quick and it all works as expected.

paulyoung · 2015-01-08T08:45:58Z

Quick/Reporting/XCTestObservationCenter+Reporting.m

+ (void)load {
+    static dispatch_once_t onceToken;
+    dispatch_once(&onceToken, ^{
+        [self swizzleInstanceMethod:@selector(_testSuiteDidStart:) withMethod:@selector(qck_testSuiteDidStart:)];


Here's where I imagine we'll have a conditional to check if a custom reporter is configured. If not, do nothing and let XCTest do it's thing.

modocache · 2015-01-08T17:27:34Z

@paulyoung First of all, amazing, inspiring stuff.

At first I thought "Ew, that's a lot more swizzling than I care for"--my proposal for Kiwi, after all, didn't supplant Xcode's output, it just wrote to another file. But the prospect of only printing clear, succinct test output in Xcode is just too appealing!

Let's definitely keep that conditional, allowing users to opt out of all the swizzling. I imagine Xcode updates may break all of this, so I'd like to allow users to keep using Quick while we fix the output. I also anticipate opinionated users may be religiously opposed to reverse-engineering XCTest. We may also be able to perform checks at runtime to ensure the XCTest private APIs are what we expect them to be...? Food for thought.

The reason Quick's tests fail is because of Quick's test helpers. They suspend logging somehow and that must not be playing very nicely with the new setup.

One question: what happens when users println from within their specs, or their production code? Is that output along with the tests? I'd prefer it if it were, since I think that's what users would expect.

I was anticipating integration with Quick, so that we could pass test reporters metadata about the examples being run. Also, as you mention, it's probably tough to get nesting right, since the reporters would have to know something about examples and example groups. We'll need nesting to address #194.

Another question: is the output identical when run from the command line?

paulyoung · 2015-01-08T20:36:28Z

We may also be able to perform checks at runtime to ensure the XCTest private APIs are what we expect them to be

I'm all for additional checks and safety 🚧

what happens when users println from within their specs, or their production code? Is that output along with the tests? I'd prefer it if it were, since I think that's what users would expect.

As far as I know, this doesn't affect anything like that. I can try it out and confirm.

is the output identical when run from the command line?

Yes! I've been using rake tasks and the output from whatever reporter is configured is the same as in the console window that appears within Xcode.

Quick's tests fail ... because ... Quick's test helpers ... suspend logging somehow

I took a brief look and it appears that they suspend observation. In any case, I can look into it further.

I was anticipating integration with Quick, so that we could pass test reporters metadata about the examples being run.

I'll proceed using this approach 👍

modocache · 2015-01-08T22:28:21Z

By the way, if we can get a nyan cat output formatter we can pretty much call it a day.

paulyoung · 2015-01-09T00:56:18Z

I was keeping Mocha's list of reporters in mind!

~=[,,_,,]:3

paulyoung · 2015-01-11T07:52:51Z

what happens when users println from within their specs, or their production code? Is that output along with the tests? I'd prefer it if it were, since I think that's what users would expect.

As far as I know, this doesn't affect anything like that. I can try it out and confirm.

Confirming that this pull request does not affect println from within specs or code being tested.

paulyoung · 2015-01-11T07:56:44Z

Quick/Reporting/XCTestObservationCenter+Reporting.m

+    NSDictionary *environment = [[NSProcessInfo processInfo] environment];
+    NSString *reporter = [environment objectForKey:@"QUICK_REPORTER"];
+
+    if (!(reporter == nil || [reporter isEqualToString:@"xcode"])) {


Perhaps this should be @"xctest" instead of @"xcode".

paulyoung · 2015-01-11T08:14:31Z

Quick/Reporting/XCTestObservationCenter+Reporting.m

+
+ (void)load {
+    NSDictionary *environment = [[NSProcessInfo processInfo] environment];
+    NSString *reporter = [environment objectForKey:@"QUICK_REPORTER"];


I tried various, apparently supported ways to do this but this was the only way I could get this to work so far:

QUICK_REPORTER=nyan xcodebuild -workspace Foo.xcworkspace -scheme Foo clean test

-quickReporter=nyan accessed via NSUserDefaults would be a lot nicer.

I'm seeing that using QUICK_REPORTER=nyan does not work for the iOS test target.

This is awesome. At some point, let's have this access an attribute on QuickConfiguration, and have configuration attributes override-able via the command line.

I've never been able to get command line arguments working, so I'm afraid I don't have any advice on how to get things working for the iOS test target.

paulyoung · 2015-01-11T08:18:57Z

Quick's tests now work again on this branch, whatever reporter is used.

paulyoung · 2015-01-11T08:29:03Z

We might want to take advantage of jdhealy/PrettyColors.

modocache · 2015-01-11T21:20:20Z

I like that library a lot! I think it'd be great to see 3rd-party reporters that use it. I'd prefer fewer dependencies in Quick itself. Just my 2 cents! 💵

Either that, or we maintain a separate set of reporters within the Quick org, but not part of this repo, that use PrettyColors. That'd actually be a pretty cool idea--maybe all but the most basic reporters should be in separate repositories?

modocache · 2015-01-11T21:30:53Z

Quick/Reporting/XCTestDriver.h

+//
+//  Created by Paul Young on 1/3/15.
+//  Copyright (c) 2015 Brian Ivan Gesiak. All rights reserved.
+//


nitpick: Remove these comments from all newly added files--the convention in Quick is to not include any template comments at the top of a file.

modocache · 2015-01-11T21:37:48Z

Let's move all the XCTest headers into their own directory. That directory should have a clear name indicating that they're exposing XCTest internals, and shouldn't be changed (unless Xcode updates its API). Something like XCTestHeaders, perhaps? Or just XCTest?

Also, just a note: the road this is going down is fraught with peril--we'll have to keep track of changes in XCTest over time and conditionally use headers based on the detected Xcode version. Yuck! I think it's still worth it, but let's just be aware of the dangers, and make sure Quick is still usable on any version, even if custom reporting may break sometimes.

paulyoung · 2015-01-12T04:54:30Z

I like that library a lot! I think it'd be great to see 3rd-party reporters that use it. I'd prefer fewer dependencies in Quick itself. Just my 2 cents! 💵

With you on that.

Either that, or we maintain a separate set of reporters within the Quick org, but not part of this repo, that use PrettyColors. That'd actually be a pretty cool idea--maybe all but the most basic reporters should be in separate repositories?

Something like the nyan cat reporter probably falls into this category; might need external dependencies and is really a novelty that would add to the size of the project if it's part of the core 🌈 🐱

Let's move all the XCTest headers into their own directory. That directory should have a clear name indicating that they're exposing XCTest internals, and shouldn't be changed (unless Xcode updates its API). Something like XCTestHeaders, perhaps? Or just XCTest?

I think there are a few things to consider here:

Some XCTest header files are already public and can be imported (for example, I'm doing this with <XCTest/XCTestRun.h>).
The header files that are available in XCTest.framework/Headers have better type information than those from class-dump.
In cases where types were unknown, I manually verified the types and used those in the implementation of the category methods (notice the changes from XCTestRun to XCTestSuiteRun and XCTestCaseRun, unsigned long long to NSUInteger, etc.
Where do we put the categories? (XCTestObservationCenter+Reporting.{h,m}).

Also, just a note: the road this is going down is fraught with peril--we'll have to keep track of changes in XCTest over time and conditionally use headers based on the detected Xcode version. Yuck! I think it's still worth it, but let's just be aware of the dangers, and make sure Quick is still usable on any version, even if custom reporting may break sometimes.

Totally. I support any decision you want to make about not doing anything too crazy. FWIW I agree that it's not ideal and hope we can come up with a better solution at some point.

Conflicts: Quick.xcodeproj/project.pbxproj

* Add TestObservationCenter. * Add Test Observer protocol. * Add DotTestReporter. * Add reporter to World and Configuration. * Move reporter check to swizzled methods due to timing of XCTestObservationCenter’s +load vs QuickConfiguration.

paulyoung · 2015-04-09T04:08:37Z

Some updates:

This branch is now up to date with master.
TestObservationCenter and TestObserver were added (modeled after the same in XCTest.framework)
A basic DotTestReporter was added.

Reporters can now be configured via QuickConfiguration:

class ProjectDataTestConfiguration: QuickConfiguration {
    override class func configure(configuration: Configuration) {
        configuration.reporter = DotTestReporter()
    }
}

Attempting to specify a reporter via the command line was removed (for now).

paulyoung · 2015-04-09T04:08:57Z

I actually have other test reporters that I haven't shared here. The reason being is that calling TestObservationCenter from the swizzled methods of XCTestObservationCenter means we can only get information as XCTest sees it and not Quick.

As a result, while I've come very close to producing BDD output that looks like the examples and groups we're familiar with it's proving overly complicated and difficult.

To remedy this I believe that we'd need to change where TestNotificationCenter methods are called from. I've identified -recordFailureWithDescription:inFile:atLine:expected: in QuickSpec.m as where I think the call site for test failures can be but I'm failing to see equivalent methods for when test suites and cases are started/stopped.

paulyoung · 2015-04-09T04:27:39Z

I forgot to add that something must have changed recently regarding ~~test suspension~~ suspending test observation.

Specifying the DotTestReporter ~~for Quick itself~~ currently causes there to be no output due to the private ivar _isSuspended always having a value of YES.

paulyoung · 2015-04-09T08:12:18Z

Quick/Reporting/XCTestObservationCenter+Reporting.m

+}
+
+- (BOOL)qck_isSuspended {
+    return (BOOL)[self valueForKey:@"_isSuspended"];


This always returns YES now, even on projects other than Quick that aren't calling private methods to suspend observation.

paulyoung · 2015-04-09T08:26:57Z

From kiwi-bdd/Kiwi/pull/449:

KWExample informs the reporter whenever it passes, fails, etc

I think that we need do to this, basically.

paulyoung · 2015-04-13T02:15:38Z

Minor update - I've created a C function which wraps Objective-C's @try { ... } @catch { ... } @finally { ... } which is callable from Swift.

I'm then calling the example closure inside of the try parameter but catch isn't being called when tests fail (using XCTAssert).

If I raise an NSException in an example, that does trigger catch.

@modocache @jeffh any ideas?

jeffh · 2015-04-13T21:47:25Z

XCTAssert raises exceptions only when XCTestCase.continueAfterFailure = false.

…onals Add equality matcher for collections of optionals

karapigeon · 2016-08-11T00:52:30Z

What's the status of this?

modocache · 2016-08-11T13:47:28Z

Sorry to have let this languish, @paulyoung. This is awesome work, and I'm feeling pretty bullish about it, so I'd love to merge and release it.

Just to confirm: is this opt-in? Or will Quick users see this output by default? And which of those options do you think is best and why?

paulyoung · 2016-08-20T00:39:28Z

@istx25 I think this was basically blocked by something related to being unable to figure out how to properly handle test failures.

It's been a while so I don't remember the details I'm afraid, but I imagine they should be pretty obvious if someone were to try to run this.

I'm not sure if/how any of these APIs have changed since I did this either.

karapigeon · 2016-08-20T06:47:38Z

Thanks for getting back to us, @paulyoung. Would you be able to run this and compile a list of things that we have to do? If APIs have changed or this is no longer relevant then we'll know. Looking forward to hearing from you. :)

paulyoung · 2016-08-22T16:44:23Z

@istx25 – I don't work on any Apple platforms at the moment, so don't have an active developer account or even have Xcode installed.

I might get around to it at the weekend but can't promise anything.

karapigeon · 2016-08-23T19:44:43Z

Thank you. No problem - Whatever you can is great.

karapigeon · 2016-10-19T10:04:45Z

Any progress on this?

karapigeon · 2016-12-12T18:57:38Z

Any progress on this?

karapigeon · 2016-12-12T18:58:05Z

Are you by chance available to do this now? @paulyoung

jeffh · 2016-12-13T04:22:21Z

This is pretty old - I'm going to close this because it would probably require some decent reworking anyways. Also we can keep the topic focused on #9. Thanks for the effort @paulyoung!

paulyoung added 2 commits January 4, 2015 03:02

Silence log output but keep Xcode support.

6fbd4e5

Use method swizzling instead of categories.

69705c2

* To support reporter configuration.

paulyoung reviewed Jan 8, 2015
View reviewed changes

Fix inconsistent indentation / code formatting.

002955f

Check for a configured reporter before swizzling.

2f8365d

paulyoung reviewed Jan 11, 2015
View reviewed changes

Respect when test observation is suspended.

6f1200c

paulyoung reviewed Jan 11, 2015
View reviewed changes

Update reporter name.

d5cd14f

modocache reviewed Jan 11, 2015
View reviewed changes

paulyoung added 3 commits January 11, 2015 20:31

Add missing prefix to category methods.

fe2bf21

Add method to check if observation is suspended.

b4148d2

Remove template comments.

3ec0a21

orta mentioned this pull request Feb 10, 2015

Support non-compile time test runners specta/specta#89

Closed

orta mentioned this pull request Apr 1, 2015

Alternative for deprecated XCTestLog/Observer? specta/specta#141

Open

paulyoung added 2 commits April 7, 2015 00:47

Merge branch 'master' into reporting

f73a2e9

Conflicts: Quick.xcodeproj/project.pbxproj

Add test reporter and configuration option.

a694016

* Add TestObservationCenter. * Add Test Observer protocol. * Add DotTestReporter. * Add reporter to World and Configuration. * Move reporter check to swizzled methods due to timing of XCTestObservationCenter’s +load vs QuickConfiguration.

Remove return value used to debug test suspension.

e1dbbc9

paulyoung reviewed Apr 9, 2015
View reviewed changes

ScottPetit mentioned this pull request Jun 4, 2015

XCTestLog/XCTestObserver is deprecated ScottPetit/SPYTestLog#2

Open

NachoSoto pushed a commit to NachoSoto/Quick that referenced this pull request Jul 18, 2016

Merge pull request Quick#233 from jwfriese/equality_for_array_of_opti…

b05318f

…onals Add equality matcher for collections of optionals

karapigeon added the github-review-needs-review label Aug 11, 2016

karapigeon assigned paulyoung Aug 23, 2016

jeffh closed this Dec 13, 2016

karapigeon added github-review-rejected and removed github-review-needs-review labels Dec 13, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Reporting #233

[WIP] Reporting #233

paulyoung commented Jan 8, 2015

paulyoung commented Jan 8, 2015

paulyoung Jan 8, 2015

modocache commented Jan 8, 2015

paulyoung commented Jan 8, 2015

modocache commented Jan 8, 2015

paulyoung commented Jan 9, 2015

paulyoung commented Jan 11, 2015

paulyoung Jan 11, 2015

paulyoung Jan 11, 2015

paulyoung Jan 11, 2015

modocache Jan 11, 2015

paulyoung commented Jan 11, 2015

paulyoung commented Jan 11, 2015

modocache commented Jan 11, 2015

modocache Jan 11, 2015

modocache commented Jan 11, 2015

paulyoung commented Jan 12, 2015

paulyoung commented Apr 9, 2015

paulyoung commented Apr 9, 2015

paulyoung commented Apr 9, 2015

paulyoung Apr 9, 2015

paulyoung commented Apr 9, 2015

paulyoung commented Apr 13, 2015

jeffh commented Apr 13, 2015

karapigeon commented Aug 11, 2016

modocache commented Aug 11, 2016

paulyoung commented Aug 20, 2016

karapigeon commented Aug 20, 2016

paulyoung commented Aug 22, 2016

karapigeon commented Aug 23, 2016

karapigeon commented Oct 19, 2016

karapigeon commented Dec 12, 2016

karapigeon commented Dec 12, 2016

jeffh commented Dec 13, 2016

[WIP] Reporting #233

[WIP] Reporting #233

Conversation

paulyoung commented Jan 8, 2015

paulyoung commented Jan 8, 2015

paulyoung Jan 8, 2015

Choose a reason for hiding this comment

modocache commented Jan 8, 2015

paulyoung commented Jan 8, 2015

modocache commented Jan 8, 2015

paulyoung commented Jan 9, 2015

paulyoung commented Jan 11, 2015

paulyoung Jan 11, 2015

Choose a reason for hiding this comment

paulyoung Jan 11, 2015

Choose a reason for hiding this comment

paulyoung Jan 11, 2015

Choose a reason for hiding this comment

modocache Jan 11, 2015

Choose a reason for hiding this comment

paulyoung commented Jan 11, 2015

paulyoung commented Jan 11, 2015

modocache commented Jan 11, 2015

modocache Jan 11, 2015

Choose a reason for hiding this comment

modocache commented Jan 11, 2015

paulyoung commented Jan 12, 2015

paulyoung commented Apr 9, 2015

paulyoung commented Apr 9, 2015

paulyoung commented Apr 9, 2015

paulyoung Apr 9, 2015

Choose a reason for hiding this comment

paulyoung commented Apr 9, 2015

paulyoung commented Apr 13, 2015

jeffh commented Apr 13, 2015

karapigeon commented Aug 11, 2016

modocache commented Aug 11, 2016

paulyoung commented Aug 20, 2016

karapigeon commented Aug 20, 2016

paulyoung commented Aug 22, 2016

karapigeon commented Aug 23, 2016

karapigeon commented Oct 19, 2016

karapigeon commented Dec 12, 2016

karapigeon commented Dec 12, 2016

jeffh commented Dec 13, 2016