-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Fake Remote Config console for testing #5633
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.
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
Changes from all commits
a4c1b7d
04be952
0809e79
5e188fa
5dd5182
c087d43
c7f7b8a
01f2cb0
2441967
9039bee
db529b1
79b0e41
9745af3
8b078f4
190030c
98d3dab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,9 +30,6 @@ NS_ASSUME_NONNULL_BEGIN | |
/// Completion handler invoked by NSSessionFetcher. | ||
typedef void (^RCNConfigFetcherCompletion)(NSData *data, NSURLResponse *response, NSError *error); | ||
|
||
/// Test block used for global NSSessionFetcher. | ||
typedef void (^RCNConfigFetcherTestBlock)(RCNConfigFetcherCompletion completion); | ||
|
||
@interface RCNConfigFetch : NSObject | ||
|
||
- (instancetype)init NS_UNAVAILABLE; | ||
|
@@ -56,8 +53,8 @@ typedef void (^RCNConfigFetcherTestBlock)(RCNConfigFetcherCompletion completion) | |
/// Add the ability to update NSURLSession's timeout after a session has already been created. | ||
- (void)recreateNetworkSession; | ||
|
||
/// Sets the test block to mock the fetch response instead of performing the fetch task from server. | ||
+ (void)setGlobalTestBlock:(RCNConfigFetcherTestBlock)block; | ||
/// Provide fetchSession for tests to override. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this better! |
||
@property(nonatomic, readwrite, strong, nonnull) NSURLSession *fetchSession; | ||
|
||
NS_ASSUME_NONNULL_END | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,7 @@ | |
* limitations under the License. | ||
*/ | ||
|
||
#import "FirebaseRemoteConfig/Sources/RCNConfigFetch.h" | ||
#import "FirebaseRemoteConfig/Sources/Private/RCNConfigFetch.h" | ||
|
||
#import <FirebaseCore/FIRApp.h> | ||
#import <FirebaseCore/FIRLogger.h> | ||
|
@@ -66,8 +66,6 @@ | |
// Deprecated error code previously from FirebaseCore | ||
static const NSInteger FIRErrorCodeConfigFailed = -114; | ||
|
||
static RCNConfigFetcherTestBlock gGlobalTestBlock; | ||
|
||
#pragma mark - RCNConfig | ||
|
||
@implementation RCNConfigFetch { | ||
|
@@ -271,13 +269,7 @@ - (void)refreshInstallationsTokenWithCompletionHandler: | |
|
||
FIRLogInfo(kFIRLoggerRemoteConfig, @"I-RCN000022", @"Success to get iid : %@.", | ||
strongSelfQueue->_settings.configInstallationsIdentifier); | ||
[strongSelf | ||
getAnalyticsUserPropertiesWithCompletionHandler:^(NSDictionary *userProperties) { | ||
dispatch_async(strongSelf->_lockQueue, ^{ | ||
[strongSelf fetchWithUserProperties:userProperties | ||
completionHandler:completionHandler]; | ||
}); | ||
}]; | ||
[strongSelf doFetchCall:completionHandler]; | ||
}); | ||
}]; | ||
}; | ||
|
@@ -286,6 +278,14 @@ - (void)refreshInstallationsTokenWithCompletionHandler: | |
[installations authTokenWithCompletion:installationsTokenHandler]; | ||
} | ||
|
||
- (void)doFetchCall:(FIRRemoteConfigFetchCompletion)completionHandler { | ||
[self getAnalyticsUserPropertiesWithCompletionHandler:^(NSDictionary *userProperties) { | ||
dispatch_async(self->_lockQueue, ^{ | ||
[self fetchWithUserProperties:userProperties completionHandler:completionHandler]; | ||
}); | ||
}]; | ||
} | ||
|
||
- (void)getAnalyticsUserPropertiesWithCompletionHandler: | ||
(FIRAInteropUserPropertiesCallback)completionHandler { | ||
FIRLogDebug(kFIRLoggerRemoteConfig, @"I-RCN000060", @"Fetch with user properties completed."); | ||
|
@@ -489,23 +489,13 @@ - (void)fetchWithUserProperties:(NSDictionary *)userProperties | |
}); | ||
}; | ||
|
||
if (gGlobalTestBlock) { | ||
gGlobalTestBlock(fetcherCompletion); | ||
return; | ||
} | ||
FIRLogDebug(kFIRLoggerRemoteConfig, @"I-RCN000061", @"Making remote config fetch."); | ||
|
||
NSURLSessionDataTask *dataTask = [self URLSessionDataTaskWithContent:compressedContent | ||
completionHandler:fetcherCompletion]; | ||
[dataTask resume]; | ||
} | ||
|
||
+ (void)setGlobalTestBlock:(RCNConfigFetcherTestBlock)block { | ||
charlotteliang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
FIRLogDebug(kFIRLoggerRemoteConfig, @"I-RCN000027", | ||
@"Set global test block for NSSessionFetcher, it will not fetch from server."); | ||
gGlobalTestBlock = [block copy]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like using a test block is simpler and doesn't take a lot of test code in production. And you can customize the block in test rather than having all the test logic in production code. You can also provide flexible return value (not just success) to test different scenarios. This is used in a lot of the unit tests of remote config (search setGlobalTestBlock in tests), but I'm not sure the current state of how many of those tests are running but if we remove this, we might also have to adjust all those unit test as well. Another thing is do we want to fake fetch in integration test since in github network is available. isn't it better to actually enable fetch from prod server? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test block is only used in disabled unit tests that still need to be ported from RC v1 to v2. This implementation allows the tests to be implemented without any knowledge of the library implementation details - just a simple API of managing a dictionary. The swift_api tests are similar to these but do fetches from a prod server. This set of tests are to enable API tests that require midstream console changes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As for me, for unit tests I would prefer injecting another implementation of For integration end-to-end tests, in contrast to unit tests, I would suggest not to mock anything and use real implementations only. |
||
} | ||
|
||
- (NSString *)constructServerURL { | ||
NSString *serverURLStr = [[NSString alloc] initWithString:kServerURLDomain]; | ||
serverURLStr = [serverURLStr stringByAppendingString:kServerURLVersion]; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
// Copyright 2020 Google LLC | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
import FirebaseCore | ||
@testable import FirebaseRemoteConfig | ||
|
||
import XCTest | ||
|
||
class FakeConsoleTests: APITestBase { | ||
override func setUp() { | ||
super.setUp() | ||
fakeConsole.config = ["Key1": "Value1"] | ||
} | ||
|
||
// Contrast with testUnchangedActivateWillError in APITests.swift. | ||
func testChangedActivateWillNotError() { | ||
let expectation = self.expectation(description: #function) | ||
config.fetch { status, error in | ||
if let error = error { | ||
XCTFail("Fetch Error \(error)") | ||
} | ||
XCTAssertEqual(status, RemoteConfigFetchStatus.success) | ||
self.config.activate { error in | ||
if let error = error { | ||
print("Activate Error \(error)") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm guessing this should also be removed :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With the deprecated API whether or not there is an error on the first activate call is random depending on the state of the console - so it might be useful to keep until we can delete along with the deprecated API at the next major release. |
||
} | ||
XCTAssertEqual(self.config["Key1"].stringValue, "Value1") | ||
expectation.fulfill() | ||
} | ||
} | ||
waitForExpectations() | ||
|
||
// Simulate updating console. | ||
fakeConsole.config = ["Key1": "Value2"] | ||
|
||
let expectation2 = self.expectation(description: #function + "2") | ||
config.fetch { status, error in | ||
if let error = error { | ||
XCTFail("Fetch Error \(error)") | ||
} | ||
XCTAssertEqual(status, RemoteConfigFetchStatus.success) | ||
self.config.activate { error in | ||
XCTAssertNil(error) | ||
XCTAssertEqual(self.config["Key1"].stringValue, "Value2") | ||
expectation2.fulfill() | ||
} | ||
} | ||
waitForExpectations() | ||
} | ||
|
||
private func waitForExpectations() { | ||
let kFIRStorageIntegrationTestTimeout = 10.0 | ||
waitForExpectations(timeout: kFIRStorageIntegrationTestTimeout, | ||
handler: { (error) -> Void in | ||
if let error = error { | ||
print(error) | ||
} | ||
}) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
// Copyright 2020 Google LLC | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
#import <FirebaseRemoteConfig/FIRRemoteConfig_Private.h> | ||
#import "FetchMocks.h" | ||
#import "FirebaseRemoteConfig/Sources/RCNConfigConstants.h" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
// Copyright 2020 Google LLC | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
class FakeConsole { | ||
var config = [String: AnyHashable]() | ||
private var last = [String: AnyHashable]() | ||
|
||
init() { | ||
config = [String: AnyHashable]() | ||
} | ||
|
||
func empty() { | ||
config = [String: AnyHashable]() | ||
} | ||
|
||
func get() -> [String: AnyHashable] { | ||
if config.count == 0 { | ||
last = config | ||
return [RCNFetchResponseKeyState: RCNFetchResponseKeyStateEmptyConfig] | ||
} | ||
var state = RCNFetchResponseKeyStateNoChange | ||
if last != config { | ||
state = RCNFetchResponseKeyStateUpdate | ||
} | ||
last = config | ||
return [RCNFetchResponseKeyState: state, RCNFetchResponseKeyEntries: config] | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
// Copyright 2020 Google LLC | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
#import <Foundation/Foundation.h> | ||
#import "FirebaseRemoteConfig/Sources/Private/RCNConfigFetch.h" | ||
|
||
NS_ASSUME_NONNULL_BEGIN | ||
|
||
@interface FetchMocks : NSObject | ||
+ (RCNConfigFetch *)mockFetch:(RCNConfigFetch *)fetch; | ||
@end | ||
|
||
NS_ASSUME_NONNULL_END |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
// Copyright 2020 Google LLC | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
#import <OCMock/OCMock.h> | ||
|
||
#import "FetchMocks.h" | ||
#import "FirebaseRemoteConfig/Sources/Private/RCNConfigFetch.h" | ||
|
||
@interface RCNConfigFetch (ExposedForTest) | ||
- (void)refreshInstallationsTokenWithCompletionHandler: | ||
(FIRRemoteConfigFetchCompletion)completionHandler; | ||
- (void)doFetchCall:(FIRRemoteConfigFetchCompletion)completionHandler; | ||
@end | ||
|
||
@implementation FetchMocks | ||
|
||
+ (RCNConfigFetch *)mockFetch:(RCNConfigFetch *)fetch { | ||
RCNConfigFetch *mock = OCMPartialMock(fetch); | ||
OCMStub([mock recreateNetworkSession]).andDo(nil); | ||
OCMStub([mock refreshInstallationsTokenWithCompletionHandler:[OCMArg any]]) | ||
.andCall(mock, @selector(doFetchCall:)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reliance on private methods can make refactoring a bit more painful in the future. |
||
return mock; | ||
} | ||
|
||
@end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pod lib lint
tests don't work now that there's multiple RC test_specs. All tests are run in GHA.