Performance enhancement of GitTopLevelDir in shell pkg #2668
Conversation
…ng and then reused + added simple benchmark test
…ncement/git-top-level-cache
| @@ -53,3 +53,13 @@ func TestRunShellOutputToStderrAndStdout(t *testing.T) { | |||
| assert.True(t, strings.Contains(stderr.String(), "Terraform"), "Output directed to stderr") | |||
| assert.True(t, len(stdout.String()) == 0, "No output to stdout") | |||
| } | |||
|
|
|||
| func BenchmarkPerformanceOfGitTopLevelDir(b *testing.B) { | |||
There was a problem hiding this comment.
Hi, will be helpful to have a functional test that will validate cache usage
shell/run_shell_cmd.go
Outdated
| @@ -230,23 +230,29 @@ type CmdOutput struct { | |||
| Stderr string | |||
| } | |||
|
|
|||
| var gitTopLevelDir string = "" | |||
There was a problem hiding this comment.
Not sure if this approach will work in case of multiple git repositories
…onfig for new generic cache + added tests for generic cache as well as for the GitTopLevelDir function that uses the cache
|
@denis256 Okay, so I have added a test for the caching. Previously, the StringCache, etc. were all in the config package. The problem with that is, that I cannot use them in the shell package due to circular import. So I have moved out all of the caches except the Also, I have made the cache structs generic with type constraint since the implementation of the This approach also supports multiple git repos now. I have also updated the gist with the new test output and new benchmark. The benchmark is almost identical to the previous one. |
|
Hi. Any updates on merging this PR? Thanks |
|
Hi, |
|
@denis256 I've updated the branch and fixed the conflict |
|
This PR still misses integration tests( |
Description
Fixes #2344.
git rev-parse --show-toplevelat the beginning and then re-use the previously found git root pathTODOs
Read the Gruntwork contribution guidelines.
Update the docs.
I didn't update the docs since I am not changing any function signatures or behavior, I am just making the specified code more efficient. If I missed some document where my change chould be described or added to, please tell me.
Run the relevant tests successfully, including pre-commit checks.
Include release notes. If this PR is backward incompatible, include a migration guide.
Release Notes (draft)
GitTopLevelDir will only run
git rev-parse --show-toplevelat the beginning and then re-use the previously found git root pathAdded / Removed / Updated [X].
Gist
Gist: https://gist.github.com/RaphSku/8cb9aae84ea374de85d621b6cc6bf8f5
for the test output and benchmark