feat: update Evaluation struct to conform to encodable. Update logger. #34
Conversation
| let expectedDictionary: [String: Any] = [ | ||
| "featureKey": "feature123", | ||
| "reason": "allocated", | ||
| "bucketValue": NSNull(), |
There was a problem hiding this comment.
'nil' is not compatible with expected dictionary value type 'Any'
| func testEncodeEvaluationReturnsValidDatafileContent() throws { | ||
|
|
||
| //GIVEN | ||
| let traffic = Traffic(key: "key", segments: .plain("segment"), percentage: 13, allocation: []) |
There was a problem hiding this comment.
As for the above, please format to:
Traffic(
key: "key",
segments: .plain("segment"),
percentage: 13,
allocation: [])
Easier to read and follow current code style formating
| public func encode(to encoder: Encoder) throws { | ||
| var container = encoder.container(keyedBy: CodingKeys.self) | ||
|
|
||
| try container.encode(featureKey, forKey: .featureKey) |
There was a problem hiding this comment.
All fields which are optional should be encoded using encodeIfPresent
| @@ -217,7 +280,70 @@ public class FeaturevisorInstance { | |||
| } | |||
|
|
|||
| func getRevision() -> String { | |||
| return datafileReader.getRevision() | |||
| return self.datafileReader.getRevision() | |||
|
|
||
| // MARK: - Bucketing | ||
|
|
||
| private func getFeature(byKey featureKey: String) -> Feature? { |
There was a problem hiding this comment.
Whydo we have changes here and for getBucketKey?
| reason: .allocated | ||
| ) | ||
|
|
||
| let expectedDictionary: [String: Any] = [ |
There was a problem hiding this comment.
Put expected under // Then. Thanks in advance. I remember that last time I wrote that it should be under GIVEN instead WHEN but // THEN would be better I believe
| variableSchema: variableSchema | ||
| ) | ||
|
|
||
| let mockedTraffic: [String: Any] = [ |
There was a problem hiding this comment.
Don't use mocked as it's confusing. Rather expectedTraffic. Also move expected stuff under "THEN"
| XCTFail("Failed to decode JSON to a dictionary.") | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
You don't have "THEN" where you can assert vs expectedDictionary as now it's not being used
| error as! FeaturevisorError, | ||
| FeaturevisorError.invalidURL(string: "hf://wrong url.com") | ||
| ) | ||
| MockURLProtocol.requestHandler = { request in |
| @@ -52,23 +188,6 @@ class FeaturevisorInstanceTests: XCTestCase { | |||
| return (response, jsonString.data(using: .utf8)) | |||
| } | |||
|
|
|||
| var featurevisorOptions = InstanceOptions.default | |||
There was a problem hiding this comment.
Why we have here these changes?
| @@ -828,8 +947,18 @@ class FeaturevisorInstanceTests: XCTestCase { | |||
| Thread.sleep(forTimeInterval: 0.1) | |||
| } | |||
|
|
|||
| // THEN | |||
| XCTAssertEqual(refreshedCount, expectedRefreshCount) | |||
| MockURLProtocol.requestHandler = { request in | |||
There was a problem hiding this comment.
Why we have here these changes?
| @@ -877,28 +1006,6 @@ class FeaturevisorInstanceTests: XCTestCase { | |||
| XCTAssertEqual(isRefreshingStopped, true) | |||
| } | |||
|
|
|||
| func testSetDatafileByValidJSON() { | |||
There was a problem hiding this comment.
Why we have these changes here?
No description provided.