Skip to content
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

Handle ENV flags properly (parse the string value) #684

Closed
shahzadlone opened this issue Jul 28, 2022 · 1 comment · Fixed by #900
Closed

Handle ENV flags properly (parse the string value) #684

shahzadlone opened this issue Jul 28, 2022 · 1 comment · Fixed by #900
Assignees
Labels
area/testing Related to any test or testing suite ci/build This is issue is about the build or CI system, and the administration of it. code quality Related to improving code quality refactor This issue specific to or requires *notable* refactoring of existing codebases and components

Comments

@shahzadlone
Copy link
Member

Here is a snippet of what we have in tests/integration/utils.go:

// We use environment variables instead of flags `go test ./...` throws for all packages
//  that don't have the flag defined
_, badgerInMemory = os.LookupEnv(memoryBadgerEnvName)
_, badgerFile = os.LookupEnv(fileBadgerEnvName)
databaseDir, _ = os.LookupEnv(fileBadgerPathEnvName)
_, mapStore = os.LookupEnv(memoryMapEnvName)
_, setupOnly = os.LookupEnv(setupOnlyEnvName)
_, detectDbChanges = os.LookupEnv(detectDbChangesEnvName)
repository, repositorySpecified := os.LookupEnv(repositoryEnvName)

Here is the what os.LookupEnv does:

// LookupEnv retrieves the value of the environment variable named
// by the key. If the variable is present in the environment the
// value (which may be empty) is returned and the boolean is true.
// Otherwise the returned value will be empty and the boolean will
// be false.

In other words only checks for the existence of the env variable without checking it's value.

For example: DEFRA_DETECT_DATABASE_CHANGES=true and DEFRA_DETECT_DATABASE_CHANGES=false are treated the same.

@shahzadlone shahzadlone added area/testing Related to any test or testing suite refactor This issue specific to or requires *notable* refactoring of existing codebases and components ci/build This is issue is about the build or CI system, and the administration of it. code quality Related to improving code quality labels Jul 28, 2022
@shahzadlone shahzadlone added this to the DefraDB v0.4 milestone Jul 28, 2022
@orpheuslummis
Copy link
Contributor

I agree that it is clearer to use values (true,false) of the env variables, rather than their existence.
That said, AFAIK the status quo is achieves the same effect but by determining the boolean via existence/non-existence rather than by defining value.
Therefore, I don't it as an issue.

@fredcarle fredcarle self-assigned this Oct 16, 2022
fredcarle added a commit that referenced this issue Oct 17, 2022
Relevant issue(s)
Resolves #684

Description
This PR improved the handling of ENV flags for our integration tests.
shahzadlone pushed a commit to shahzadlone/defradb that referenced this issue Feb 23, 2024
Relevant issue(s)
Resolves sourcenetwork#684

Description
This PR improved the handling of ENV flags for our integration tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing Related to any test or testing suite ci/build This is issue is about the build or CI system, and the administration of it. code quality Related to improving code quality refactor This issue specific to or requires *notable* refactoring of existing codebases and components
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants