Added env.UserHomeDir(ctx) for parallel-friendly tests
#955
Conversation
`os.Getenv(..)` is not friendly with `libs/env`. This PR makes the relevant changes to places where we need to read user home directory.
nfx
added
the
Tech Debt
libs/env/context.go
Outdated
| } | ||
|
|
||
| func UserHomeDir(ctx context.Context) string { | ||
| return Get(ctx, homeEnvVar()) |
There was a problem hiding this comment.
So os.UserHomeDir returns an error if it is empty. This silently returns an empty string. We should return an error here as well to avoid doing the wrong thing when the home directory inadvertently is not set.
There was a problem hiding this comment.
which can be the cases when HOME is empty?
There was a problem hiding this comment.
Users may exec into an environment where it is not set, i.e. a container, or empty env.
The important bit is that we should never silently do the wrong thing, so we should either return an error from this function (reasonable), or panic if the value is empty (not great).
There was a problem hiding this comment.
The tests fail because not all code paths end up cleaning the path. To fix the tests, you can call filepath.Clean from GetPath and from env.UserHomeDir (or where the string prefix check is made).
Everything else LGTM.
The panic on missing $HOME is risky, but we can go with it until we get evidence that it doesn't hold up everywhere. I spot checked a couple bare bones Docker containers and observe a $HOME everywhere.
CLI: * Hide `--progress-format` global flag ([#965](#965)). * Make configure command visible + fix bundle command description ([#961](#961)). * Log process ID in each log entry ([#949](#949)). * Improve error message when `--json` flag is specified ([#933](#933)). Bundles: * Remove validation for default value against pattern ([#959](#959)). * Bundle path rewrites for dbt and SQL file tasks ([#962](#962)). * Initialize variable definitions that are defined without properties ([#966](#966)). Internal: * Remove mention of Lakehouse apps from the changelog ([#945](#945)). * Function to merge two instances of `config.Value` ([#938](#938)). * Make to/from string methods private to the jsonschema package ([#942](#942)). * Make Cobra runner compatible with testing interactive flows ([#957](#957)). * Added `env.UserHomeDir(ctx)` for parallel-friendly tests ([#955](#955)). Dependency updates: * Bump golang.org/x/mod from 0.13.0 to 0.14.0 ([#954](#954)). * Bump golang.org/x/text from 0.13.0 to 0.14.0 ([#953](#953)). * Bump golang.org/x/sync from 0.4.0 to 0.5.0 ([#951](#951)). * Bump github.com/spf13/cobra from 1.7.0 to 1.8.0 ([#950](#950)). * Bump github.com/fatih/color from 1.15.0 to 1.16.0 ([#952](#952)).
CLI: * Hide `--progress-format` global flag ([#965](#965)). * Make configure command visible + fix bundle command description ([#961](#961)). * Log process ID in each log entry ([#949](#949)). * Improve error message when `--json` flag is specified ([#933](#933)). Bundles: * Remove validation for default value against pattern ([#959](#959)). * Bundle path rewrites for dbt and SQL file tasks ([#962](#962)). * Initialize variable definitions that are defined without properties ([#966](#966)). Internal: * Function to merge two instances of `config.Value` ([#938](#938)). * Make to/from string methods private to the jsonschema package ([#942](#942)). * Make Cobra runner compatible with testing interactive flows ([#957](#957)). * Added `env.UserHomeDir(ctx)` for parallel-friendly tests ([#955](#955)). Dependency updates: * Bump golang.org/x/mod from 0.13.0 to 0.14.0 ([#954](#954)). * Bump golang.org/x/text from 0.13.0 to 0.14.0 ([#953](#953)). * Bump golang.org/x/sync from 0.4.0 to 0.5.0 ([#951](#951)). * Bump github.com/spf13/cobra from 1.7.0 to 1.8.0 ([#950](#950)). * Bump github.com/fatih/color from 1.15.0 to 1.16.0 ([#952](#952)).
Changes
os.Getenv(..)is not friendly withlibs/env. This PR makes the relevant changes to places where we need to read user home directory.Tests
Mainly done in #914