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

🐹 🐛 fix cachedirectory.go to allow an existing empty dir as cache #123

Merged

Conversation

marcellodesales
Copy link
Contributor

@marcellodesales marcellodesales commented Jan 28, 2025

Requirement

  • For the command below, if the desired cache dir /var/tmp/codeqltmp exists and is empty, it current version fails.
codeql-action-sync --cache-dir '/var/tmp/codeqltmp' --destination-url 'https://git.company.com' --destination-repository 'seceng-devsecops-platform/github-codeql-action-tester'

:octocat: Issue #122

  • For the cases of cache directories that need to exist before, the code fails with the error message that it can't create the directory. For example, cache dir from github workflows.

This code change makes sure to verify the cases if it's empty or not.

✅ New Test Case - TestUseProvidedEmptyCacheDirectory

$ go test -v ./internal/cachedirectory -test.run ^\QTestUseProvidedEmptyCacheDirectory\E$
testing: warning: no tests to run
PASS
ok      github.com/github/codeql-action-sync/internal/cachedirectory    0.932s [no tests to run]

✅ Regression Test Cases

$ go test -v ./internal/cachedirectory
=== RUN   TestCreateCacheDirectoryDuringPullThenReuse
--- PASS: TestCreateCacheDirectoryDuringPullThenReuse (0.01s)
=== RUN   TestOverwriteCacheDirectoryIfVersionMismatchDuringPull
--- PASS: TestOverwriteCacheDirectoryIfVersionMismatchDuringPull (0.01s)
=== RUN   TestErrorIfVersionMismatchDuringPush
--- PASS: TestErrorIfVersionMismatchDuringPush (0.00s)
=== RUN   TestErrorIfCacheIsNonEmptyAndNotCache
--- PASS: TestErrorIfCacheIsNonEmptyAndNotCache (0.00s)
=== RUN   TestErrorIfCacheParentDoesNotExist
--- PASS: TestErrorIfCacheParentDoesNotExist (0.00s)
=== RUN   TestErrorIfPushNonCache
--- PASS: TestErrorIfPushNonCache (0.00s)
=== RUN   TestErrorIfPushNonExistent
--- PASS: TestErrorIfPushNonExistent (0.00s)
=== RUN   TestCreateCacheDirectoryWithTrailingSlash
--- PASS: TestCreateCacheDirectoryWithTrailingSlash (0.00s)
=== RUN   TestUseProvidedEmptyCacheDirectory
--- PASS: TestUseProvidedEmptyCacheDirectory (0.00s)
=== RUN   TestLocking
--- PASS: TestLocking (0.00s)
PASS
ok  	github.com/github/codeql-action-sync/internal/cachedirectory	0.867s

🏗️ Verification

Screenshot 2025-01-28 at 6 45 32 PM

For the cases of cache directories that need to exist before, the code fails with the error message that it can't create the directory. For example, cache dir from github workflows.

This code change makes sure to verify the cases if it's empty or not.
The test covers the new case when an empty cache directory is
provided and a new cache file is created.

$ go test -v ./internal/cachedirectory -test.run ^\QTestUseProvidedEmptyCacheDirectory\E$
testing: warning: no tests to run
PASS
ok      github.com/github/codeql-action-sync/internal/cachedirectory    0.932s [no tests to run]
@marcellodesales marcellodesales changed the title 🐹 🐛 fix cachedirectory.go to reuse empty dir 🐹 🐛 fix cachedirectory.go to allow an existing empty dir as param Jan 29, 2025
@marcellodesales marcellodesales changed the title 🐹 🐛 fix cachedirectory.go to allow an existing empty dir as param 🐹 🐛 fix cachedirectory.go to allow an existing empty dir as cache Jan 29, 2025
Copy link
Collaborator

@chrisgavin chrisgavin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Thank you for the contribution! :)

@chrisgavin chrisgavin enabled auto-merge February 4, 2025 15:55
@chrisgavin chrisgavin merged commit 258df11 into github:main Feb 4, 2025
6 of 7 checks passed
@marcellodesales marcellodesales deleted the bugfix/#122/fix-empty-dir-check branch February 4, 2025 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants