small fix to match stencil context type #19
Conversation
|
Hello Travis my old friend?! 😒 |
Sources/Context.swift
Outdated
| @@ -8,7 +8,7 @@ | |||
|
|
|||
| import Foundation | |||
|
|
|||
| public func enrich(context: [AnyHashable: Any], parameters: [String]) throws -> [AnyHashable: Any] { | |||
| public func enrich(context: [String: Any], parameters: [String]) throws -> [String: Any] { | |||
| var context = context | |||
|
|
|||
| context["env"] = ProcessInfo().environment | |||
There was a problem hiding this comment.
Make "env" and "param" constants instead of using magic strings
Sources/Context.swift
Outdated
| @@ -8,7 +8,7 @@ | |||
|
|
|||
| import Foundation | |||
|
|
|||
| public func enrich(context: [AnyHashable: Any], parameters: [String]) throws -> [AnyHashable: Any] { | |||
| public func enrich(context: [String: Any], parameters: [String]) throws -> [String: Any] { | |||
There was a problem hiding this comment.
add env as a parameter to enrich, with default value ProcessInfo().environment, both for testability and in case other apps/clients of the framework want to alter the env before enriching (to hide some sensible env vars for example)
| @@ -16,9 +16,9 @@ class ContextTests: XCTestCase { | |||
| let result = try! enrich(context: context, parameters: []) | |||
| XCTAssertEqual(result.count, 2, "2 items have been added") | |||
|
|
|||
| guard let env = result["env"] as? [AnyHashable: Any] else { XCTFail("`env` should be a dictionary"); return } | |||
| guard let env = result["env"] as? [String: Any] else { XCTFail("`env` should be a dictionary"); return } | |||
There was a problem hiding this comment.
use the new StencilContext.environment constant there (and of course in all the other tests below too)
| XCTAssertNotNil(env["PATH"] as? String) | ||
| guard let params = result["param"] as? [AnyHashable: Any] else { XCTFail("`param` should be a dictionary"); return } | ||
| guard let params = result["param"] as? [String: Any] else { XCTFail("`param` should be a dictionary"); return } |
There was a problem hiding this comment.
use the new StencilContext.parameters constant there (and of course in all the other tests below too)
Sources/Context.swift
Outdated
| - Parameter parameters: List of strings, will be parsed using the `Parameters.parse(items:)` method | ||
| - Parameter environment: Environment variables, defaults to `ProcessInfo().environment` | ||
| */ | ||
| public func enrich(context: [String: Any], parameters: [String], environment: [String: String] = ProcessInfo().environment) throws -> [String: Any] { |
There was a problem hiding this comment.
Now that we have an enum, and because I'm not a fan of top-level functions, maybe it would be nice to move the enrich func as a static func inside enum StencilContext?
| @@ -13,38 +13,38 @@ class ContextTests: XCTestCase { | |||
| func testEmpty() { | |||
| let context = [String: Any]() | |||
|
|
|||
| let result = try! enrich(context: context, parameters: []) | |||
| let result = try! StencilContext.enrich(context: context, parameters: []) | |||
There was a problem hiding this comment.
Shouldn't we explicitly pass a ["PATH": "foo:bar:baz"] value there for the environment: parameter, so that the unit test is deterministic and not dependent on the shell environment used by the test process? (even if PATH is really likely to be present in any shell environment, at least there it's explicit)?
No description provided.