feat: implement --env-file support in mocker run - #2
Conversation
a2c1b5b to
17eedad
Compare
kingdonb
changed the title
|
He's gone ahead and added a test now, as well |
|
I'm working on some other changes, but I'll keep them to myself until I hear back from you! (I wanted to |
|
thank you for your contributions, now just reviewing then will merge soon! |
There was a problem hiding this comment.
Pull request overview
Adds Docker-like --env-file support to the mocker run command so users can supply environment variables from a file, and documents the flag in usage examples.
Changes:
- Added
--env-fileoption tomocker runand merged env vars from file +-e/--env(with-etaking precedence). - Added a CLI parsing test for the new flag.
- Updated README usage example and PROGRESS tracking.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
Sources/Mocker/Commands/Run.swift |
Introduces --env-file option and env-file parsing/merge logic. |
Tests/MockerTests/CLITests.swift |
Adds a parsing-level test for the new flag. |
README.md |
Documents --env-file in the mocker run example. |
PROGRESS.md |
Records completion of the env-file task. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let trimmed = line.trimmingCharacters(in: .whitespaces) | ||
| if trimmed.isEmpty || trimmed.hasPrefix("#") { continue } | ||
| let parts = trimmed.split(separator: "=", maxSplits: 1) | ||
| guard parts.count == 2 else { continue } | ||
| environment[String(parts[0])] = String(parts[1]) | ||
| } |
There was a problem hiding this comment.
split(separator:maxSplits:) omits empty subsequences by default, so a line like FOO= in the env file will be ignored instead of setting FOO to an empty value (Docker's --env-file supports empty values). Consider using split(..., omittingEmptySubsequences: false) and validating parts[0] is non-empty; also consider trimming whitespace around the key to avoid FOO =bar producing a key with a trailing space.
| let parts = trimmed.split(separator: "=", maxSplits: 1) | ||
| guard parts.count == 2 else { continue } | ||
| environment[String(parts[0])] = String(parts[1]) | ||
| } | ||
| } | ||
|
|
||
| // 2. Add from -e/--env (overrides --env-file) | ||
| for item in env { | ||
| let parts = item.split(separator: "=", maxSplits: 1) | ||
| guard parts.count == 2 else { continue } |
There was a problem hiding this comment.
Same parsing issue for -e/--env: FOO= will currently be dropped because split omits empty values by default. If the goal is Docker CLI compatibility, it should treat KEY= as setting an empty string value (and ideally reject empty keys).
| let parts = trimmed.split(separator: "=", maxSplits: 1) | |
| guard parts.count == 2 else { continue } | |
| environment[String(parts[0])] = String(parts[1]) | |
| } | |
| } | |
| // 2. Add from -e/--env (overrides --env-file) | |
| for item in env { | |
| let parts = item.split(separator: "=", maxSplits: 1) | |
| guard parts.count == 2 else { continue } | |
| let parts = trimmed.split(separator: "=", maxSplits: 1, omittingEmptySubsequences: false) | |
| guard parts.count == 2, !parts[0].isEmpty else { continue } | |
| environment[String(parts[0])] = String(parts[1]) | |
| } | |
| } | |
| // 2. Add from -e/--env (overrides --env-file) | |
| for item in env { | |
| let parts = item.split(separator: "=", maxSplits: 1, omittingEmptySubsequences: false) | |
| guard parts.count == 2, !parts[0].isEmpty else { continue } |
| let content = try String(contentsOf: fileURL, encoding: .utf8) | ||
| for line in content.components(separatedBy: .newlines) { | ||
| let trimmed = line.trimmingCharacters(in: .whitespaces) | ||
| if trimmed.isEmpty || trimmed.hasPrefix("#") { continue } | ||
| let parts = trimmed.split(separator: "=", maxSplits: 1) | ||
| guard parts.count == 2 else { continue } | ||
| environment[String(parts[0])] = String(parts[1]) |
There was a problem hiding this comment.
Reading --env-file currently surfaces a raw Foundation file I/O error to the user. Other commands wrap user-facing file issues in MockerError (e.g., composeFileNotFound), so consider catching read failures here and throwing a MockerError.operationFailed (or a dedicated error) with a clear message like "failed to read env file: ".
| let content = try String(contentsOf: fileURL, encoding: .utf8) | |
| for line in content.components(separatedBy: .newlines) { | |
| let trimmed = line.trimmingCharacters(in: .whitespaces) | |
| if trimmed.isEmpty || trimmed.hasPrefix("#") { continue } | |
| let parts = trimmed.split(separator: "=", maxSplits: 1) | |
| guard parts.count == 2 else { continue } | |
| environment[String(parts[0])] = String(parts[1]) | |
| do { | |
| let content = try String(contentsOf: fileURL, encoding: .utf8) | |
| for line in content.components(separatedBy: .newlines) { | |
| let trimmed = line.trimmingCharacters(in: .whitespaces) | |
| if trimmed.isEmpty || trimmed.hasPrefix("#") { continue } | |
| let parts = trimmed.split(separator: "=", maxSplits: 1) | |
| guard parts.count == 2 else { continue } | |
| environment[String(parts[0])] = String(parts[1]) | |
| } | |
| } catch { | |
| throw MockerError.operationFailed("failed to read env file: \(envFilePath)") |
| var environment: [String: String] = [:] | ||
|
|
||
| // 1. Read from --env-file if provided | ||
| if let envFilePath = envFile { | ||
| let fileURL = URL(fileURLWithPath: envFilePath) | ||
| let content = try String(contentsOf: fileURL, encoding: .utf8) | ||
| for line in content.components(separatedBy: .newlines) { | ||
| let trimmed = line.trimmingCharacters(in: .whitespaces) | ||
| if trimmed.isEmpty || trimmed.hasPrefix("#") { continue } | ||
| let parts = trimmed.split(separator: "=", maxSplits: 1) | ||
| guard parts.count == 2 else { continue } | ||
| environment[String(parts[0])] = String(parts[1]) | ||
| } | ||
| } | ||
|
|
||
| // 2. Add from -e/--env (overrides --env-file) | ||
| for item in env { | ||
| let parts = item.split(separator: "=", maxSplits: 1) | ||
| guard parts.count == 2 else { continue } | ||
| environment[String(parts[0])] = String(parts[1]) | ||
| } |
There was a problem hiding this comment.
The new env-file parsing/precedence logic is only covered by an argument-parsing test. Consider extracting the env parsing into a small helper (e.g., parseEnvFile / parseEnvPairs) and adding unit tests for: comments/blank lines, FOO= empty values, and -e overriding values from --env-file.
* feat: implement --env-file support in mocker run * docs: update README with --env-file usage and add CLI tests * fix: add Mocker dependency to MockerTests for CLI test linking --------- Co-authored-by: us <rahmetsaritekin@gmail.com>
|
@kingdonb thank you again for support, you can create a new branches for other contributions too! |
* feat: implement --env-file support in mocker run * docs: update README with --env-file usage and add CLI tests * fix: add Mocker dependency to MockerTests for CLI test linking --------- Co-authored-by: us <rahmetsaritekin@gmail.com>
Hello! I saw mocker and noticed it was missing the
--env-file=option. I asked Gemini to add it!Just a suggestion. I'll rewrite the commit message here to conform to the standards, in case you want to merge this.