Skip to content

Commit 248134b

Browse files
committed
make ReportMessage take a list of reporters instead of using globals
Having the global list of reporters is holding us back in PR facebookarchive#51. @yiding is parallelizing test runs, buffering their reporter events until the test is finished, and then flushing all the reporter events at at the end. In this case, we need the events from ReportMessage(...) to feed into the BufferedReporters intead of sidestepping it and going for the globals. Tests pass, and via manual testing verified that reporter messages show up correctly.
1 parent f6618fd commit 248134b

File tree

5 files changed

+18
-36
lines changed

5 files changed

+18
-36
lines changed

xctool/xctool/OCUnitIOSAppTestRunner.m

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -222,14 +222,16 @@ - (BOOL)runTestsAndFeedOutputTo:(void (^)(NSString *))outputLineBlock error:(NSS
222222
NSString *testHostBundleID = plist[@"CFBundleIdentifier"];
223223

224224
if (_freshSimulator) {
225-
ReportMessage(REPORTER_MESSAGE_INFO,
225+
ReportMessage(_reporters,
226+
REPORTER_MESSAGE_INFO,
226227
@"Stopping any existing iOS simulator jobs to get a "
227228
@"fresh simulator.");
228229
KillSimulatorJobs();
229230
}
230231

231232
if (_freshInstall) {
232-
ReportMessage(REPORTER_MESSAGE_INFO,
233+
ReportMessage(_reporters,
234+
REPORTER_MESSAGE_INFO,
233235
@"Uninstalling '%@' to get a fresh install.",
234236
testHostBundleID);
235237
BOOL uninstalled = [self runMobileInstallationHelperWithArguments:@[
@@ -255,7 +257,7 @@ - (BOOL)runTestsAndFeedOutputTo:(void (^)(NSString *))outputLineBlock error:(NSS
255257
//
256258
// By making sure the app is already installed, we guarantee the environment
257259
// is always set correctly.
258-
ReportMessage(REPORTER_MESSAGE_INFO, @"Installing '%@' ...", testHostAppPath);
260+
ReportMessage(_reporters, REPORTER_MESSAGE_INFO, @"Installing '%@' ...", testHostAppPath);
259261
BOOL installed = [self runMobileInstallationHelperWithArguments:@[
260262
@"install",
261263
testHostAppPath,
@@ -267,7 +269,8 @@ - (BOOL)runTestsAndFeedOutputTo:(void (^)(NSString *))outputLineBlock error:(NSS
267269
return NO;
268270
}
269271

270-
ReportMessage(REPORTER_MESSAGE_INFO,
272+
ReportMessage(_reporters,
273+
REPORTER_MESSAGE_INFO,
271274
@"Launching test host and running tests...");
272275
if (![self runTestsInSimulator:testHostAppPath feedOutputToBlock:outputLineBlock]) {
273276
*error = [NSString stringWithFormat:@"Failed to run tests"];

xctool/xctool/Options.m

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -258,11 +258,15 @@ - (BOOL)validateOptions:(NSString **)errorMessage
258258
}
259259

260260
if (targetMatch.workspacePath) {
261-
ReportMessage(REPORTER_MESSAGE_INFO,
261+
ReportMessage(
262+
_reporters,
263+
REPORTER_MESSAGE_INFO,
262264
@"Found target %@. Using workspace path %@, scheme %@.",
263265
self.findTarget, targetMatch.workspacePath, targetMatch.schemeName);
264266
} else {
265-
ReportMessage(REPORTER_MESSAGE_INFO,
267+
ReportMessage(
268+
_reporters,
269+
REPORTER_MESSAGE_INFO,
266270
@"Found target %@. Using project path %@, scheme %@.",
267271
self.findTarget, targetMatch.projectPath, targetMatch.schemeName);
268272
}

xctool/xctool/Reporter.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,7 @@ typedef enum {
116116

117117
NSString *ReporterMessageLevelToString(ReporterMessageLevel level);
118118

119-
void RegisterReporters(NSArray *reporters);
120-
void UnregisterReporters(NSArray *reporters);
121-
122-
void ReportMessage(ReporterMessageLevel level, NSString *format, ...) NS_FORMAT_FUNCTION(2, 3);
119+
void ReportMessage(NSArray *reporters, ReporterMessageLevel level, NSString *format, ...) NS_FORMAT_FUNCTION(3, 4);
123120

124121
@interface Reporter : NSObject
125122
{

xctool/xctool/Reporter.m

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -39,25 +39,11 @@
3939
}
4040
}
4141

42-
static NSArray *RegisteredReporters = nil;
43-
44-
void RegisterReporters(NSArray *reporters) {
45-
NSCAssert(RegisteredReporters == nil, @"Cannot register reporters twice.");
46-
NSCAssert(reporters != nil, @"Must have array of reporters to register.");
47-
RegisteredReporters = [reporters retain];
48-
}
49-
50-
void UnregisterReporters(NSArray *reporters) {
51-
NSCAssert(reporters == RegisteredReporters, @"Cannot unregister reporters without first registering.");
52-
[RegisteredReporters release];
53-
RegisteredReporters = nil;
54-
}
55-
56-
void ReportMessage(ReporterMessageLevel level, NSString *format, ...) {
42+
void ReportMessage(NSArray *reporters, ReporterMessageLevel level, NSString *format, ...) {
5743
va_list args;
5844
va_start(args, format);
59-
6045
NSString *message = [[[NSString alloc] initWithFormat:format arguments:args] autorelease];
46+
va_end(args);
6147

6248
NSDate *now = [NSDate date];
6349
NSDictionary *event = @{
@@ -67,10 +53,8 @@ void ReportMessage(ReporterMessageLevel level, NSString *format, ...) {
6753
kReporter_Message_LevelKey: ReporterMessageLevelToString(level),
6854
};
6955

70-
for (Reporter *reporter in RegisteredReporters) {
71-
[reporter message:event];
72-
}
73-
va_end(args);
56+
[reporters makeObjectsPerformSelector:@selector(handleEvent:)
57+
withObject:event];
7458
}
7559

7660
@implementation Reporter

xctool/xctool/XCTool.m

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -165,9 +165,6 @@ - (void)run
165165
}
166166
}
167167

168-
// After this point, we can start reporting messages.
169-
RegisterReporters(options.reporters);
170-
171168
// We want to make sure we always unregister the reporters, even if validation fails,
172169
// so we use a try-finally block.
173170
@try {
@@ -205,9 +202,6 @@ - (void)run
205202
}
206203
} @finally {
207204
[options.reporters makeObjectsPerformSelector:@selector(close)];
208-
209-
// After this point, we can no longer report messages.
210-
UnregisterReporters(options.reporters);
211205
}
212206
}
213207

0 commit comments

Comments
 (0)