From 73cdc1b4e46a73e0e8dea7257eeb4ab7066fbb71 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Wed, 16 May 2018 08:33:28 -0700 Subject: [PATCH] Call out to delegate for control users in experiments (#923) --- CHANGELOG.md | 1 + Source/ASConfigurationInternal.m | 7 ++++--- Tests/ASConfigurationTests.m | 10 ++++------ 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cdcc3f106..14738bb5a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ - Adds an experiment to call ASNetworkImageNode callbacks off main. [Garrett Moon](https://github.com/garrettmoon) - Prevent UITextView from updating contentOffset while deallocating [Michael Schneider](https://github.com/maicki) - [ASCollectionNode/ASTableNode] Fix a crash occurs while remeasuring cell nodes. [Huy Nguyen](https://github.com/nguyenhuy) [#917](https://github.com/TextureGroup/Texture/pull/917) +- Fix an issue where ASConfigurationDelegate would not call out for "control" users. If set, it now receives events whenever an experimental feature decision point occurs, whether it's enabled or not. [Adlai Holler](https://github.com/Adlai-Holler) ## 2.6 - [Xcode 9] Updated to require Xcode 9 (to fix warnings) [Garrett Moon](https://github.com/garrettmoon) diff --git a/Source/ASConfigurationInternal.m b/Source/ASConfigurationInternal.m index a11e242290..9d7c1bcf5a 100644 --- a/Source/ASConfigurationInternal.m +++ b/Source/ASConfigurationInternal.m @@ -63,10 +63,11 @@ NSAssert(__builtin_popcount(requested) == 1, @"Cannot activate multiple features at once with this method."); - // If they're disabled, ignore them. + // We need to call out, whether it's enabled or not. + // A/B testing requires even "control" users to be activated. ASExperimentalFeatures enabled = requested & _config.experimentalFeatures; - ASExperimentalFeatures prevTriggered = atomic_fetch_or(&_activatedExperiments, enabled); - ASExperimentalFeatures newlyTriggered = enabled & ~prevTriggered; + ASExperimentalFeatures prevTriggered = atomic_fetch_or(&_activatedExperiments, requested); + ASExperimentalFeatures newlyTriggered = requested & ~prevTriggered; // Notify delegate if needed. if (newlyTriggered != 0) { diff --git a/Tests/ASConfigurationTests.m b/Tests/ASConfigurationTests.m index 41e0d0ed81..cd7ed1034e 100644 --- a/Tests/ASConfigurationTests.m +++ b/Tests/ASConfigurationTests.m @@ -33,14 +33,11 @@ [ASConfigurationManager test_resetWithConfiguration:config]; // Set an expectation for a callback, and assert we only get one. - XCTestExpectation *e = [self expectationWithDescription:@"Callback 1 done."]; + XCTestExpectation *e = [self expectationWithDescription:@"Callbacks done."]; + e.expectedFulfillmentCount = 2; + e.assertForOverFulfill = YES; onActivate = ^(ASConfigurationTests *self, ASExperimentalFeatures feature) { - XCTAssertEqual(feature, ASExperimentalGraphicsContexts); [e fulfill]; - // Next time it's a fail. - self->onActivate = ^(ASConfigurationTests *self, ASExperimentalFeatures feature) { - XCTFail(@"Too many callbacks."); - }; }; // Now activate the graphics experiment and expect it works. @@ -48,6 +45,7 @@ // We should get a callback here // Now activate text node and expect it fails. XCTAssertFalse(ASActivateExperimentalFeature(ASExperimentalTextNode)); + // But we should get another callback. [self waitForExpectationsWithTimeout:3 handler:nil]; }