Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Always call out to delegate for experiments, whether enabled or not #923

Merged
merged 1 commit into from
May 16, 2018
Merged

Always call out to delegate for experiments, whether enabled or not #923

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 4 additions & 3 deletions Source/ASConfigurationInternal.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,11 @@ - (BOOL)activateExperimentalFeature:(ASExperimentalFeatures)requested

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) {
Expand Down
10 changes: 4 additions & 6 deletions Tests/ASConfigurationTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,19 @@ - (void)testExperimentalFeatureConfig
[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.
XCTAssertTrue(ASActivateExperimentalFeature(ASExperimentalGraphicsContexts));
// 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];
}

Expand Down