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

Jump to bottom

os: UserCacheDir and UserConfigDir have no tests #57638

Closed
bcmills opened this issue Jan 5, 2023 · 4 comments · May be fixed by #57809
Closed

os: UserCacheDir and UserConfigDir have no tests #57638

bcmills opened this issue Jan 5, 2023 · 4 comments · May be fixed by #57809
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Backlog

Comments

@bcmills
Copy link
Contributor

bcmills commented Jan 5, 2023

os.UserCacheDir was added in https://go.dev/cl/78835 for #22536.

In examining #57630, it seems that this function is not covered by any tests at all. There were none added in https://go.dev/cl/78835, nor in https://go.dev/cl/120757.

It seems like we should at least have some test that verifies that UserCacheDir reports either an error or a directory in which the user is allowed to create files. 😅

(CC @rsc @robpike @ianlancetaylor @bradfitz @griesemer for os)
@bcmills bcmills added Testing An issue that has been verified to require only test changes, not just a test failure. help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Jan 5, 2023
@bcmills bcmills added this to the Backlog milestone Jan 5, 2023
@bcmills bcmills mentioned this issue Jan 5, 2023
Closed
@gopherbot
Copy link

Change https://go.dev/cl/460917 mentions this issue: gopls/internal/lsp/filecache: use TempDir unless MkdirAll(UserCacheDir)

gopherbot pushed a commit to golang/tools that referenced this issue Jan 6, 2023
@adonovan
gopls/internal/lsp/filecache: use TempDir if UserCacheDir fails us
2eb6138 
On the builders, HOME=/ which causes UserCacheDir to be non-writable.
This change attempts to MkdirAll the UserCacheDir and, if that fails,
we fail back to TempDir, which is assumed to exist and be writable.

Updates golang/go#57638

Change-Id: I6eb81c59b50f90a62c103a2511425fa2f24452fa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/460917
Reviewed-by: Bryan Mills <bcmills@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@Ja7ad Ja7ad mentioned this issue Jan 16, 2023
Open
Ja7ad added a commit to Ja7ad/go that referenced this issue Jan 17, 2023
@Ja7ad
os: ignored is not exist error
8cb19f0 
Fixes golang#57638
Ja7ad added a commit to Ja7ad/go that referenced this issue Jan 18, 2023
@Ja7ad
os: update test use cache dir
0f183ac 
error UserCacheDir skipped and empty string cache dir got fatal.

for test write permission in cache dir add mkdir temp.

Fixes golang#57638 golang#57630
@gopherbot
Copy link

Change https://go.dev/cl/462041 mentions this issue: os: add test for UserCacheDir

@bcmills
Copy link
Contributor Author

bcmills commented Jun 26, 2023

It appears that UserConfigDir (https://go.dev/cl/160877 / #29960) is similarly untested.

@bcmills bcmills changed the title os: UserCacheDir has no tests os: UserCacheDir and UserConfigDir have no tests Jun 26, 2023
@bcmills bcmills mentioned this issue Jun 26, 2023
Open
@gopherbot
Copy link

Change https://go.dev/cl/520262 mentions this issue: os: add tests for UserCacheDir and UserConfigDir

@bcmills bcmills mentioned this issue Aug 18, 2023
Open
This was referenced Aug 22, 2023
Closed
Closed
cellularmitosis pushed a commit to cellularmitosis/go that referenced this issue Aug 24, 2023
@joshkraft @cellularmitosis
os: add tests for UserCacheDir and UserConfigDir
9032e4c 
Adds basic test scenarios for UserCacheDir and
UserConfigDir.

Fixes golang#57638

Change-Id: Ieb86e95faff44287bfa13daa0cb26e7b5401373b
Reviewed-on: https://go-review.googlesource.com/c/go/+/520262
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Bypass: Bryan Mills <bcmills@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
@zhsj zhsj mentioned this issue Jan 6, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

os: add test for UserCacheDir Ja7ad/go
2 participants
@bcmills @gopherbot