studio/flamenco
studio/flamenco
12
30
Fork 38
Code Issues 63 Pull Requests 5 Releases Wiki Activity

Shaman: fail unit test when running as root user (linux) #104234

Manually merged
Sybren A. Stüvel merged 2 commits from michael-2/flamenco:mgr_api_meta_test_TestCheckSharedStoragePath into main 2023-07-07 16:07:17 +02:00
Conversation 1 Commits 2 Files Changed 1 +19 -12
1 changed files with 19 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 4deef05cf9 - Show all commits

31
internal/manager/api_impl/meta_test.go
View File

@ -6,6 +6,7 @@ import (
"io/fs" "io/fs"
"net/http" "net/http"
"os" "os"
"os/user"
"path/filepath" "path/filepath"
"runtime" "runtime"
"testing" "testing"
@ -189,20 +190,26 @@ func TestCheckSharedStoragePath(t *testing.T) {
// that seems consistent. // that seems consistent.
// FIXME: find another way to test with unwritable directories on Windows. // FIXME: find another way to test with unwritable directories on Windows.
if runtime.GOOS != "windows" { if runtime.GOOS != "windows" {
parentPath := filepath.Join(mf.tempdir, "deep") currentUser, err := user.Current()
testPath := filepath.Join(parentPath, "nesting")
if err := os.Mkdir(parentPath, fs.ModePerm); !assert.NoError(t, err) { if err != nil {
t.FailNow() t.FailNow()
Sybren A. Stüvel commented 2023-07-06 10:46:26 +02:00
Outdated
Review
Copy Link

I feel there's too much nesting going on here. Also t.FailNow() wouldn't show the actual content of err. Finally, the original code was written before I realised I could do require.NoError(...) instead of assert.NoError(...) and it would immediately stop the test.

This would be better:

// Root can always create directories, so this test would fail with a
// confusing message. Instead it's better to refuse running as root at all.
currentUser, err := user.Current()
require.NoError(t, err)
require.NotEqual(t, "0", currentUser.Uid,
	"this test requires running as normal user, not %s (%s)", currentUser.Username, currentUser.Uid)
require.NotEqual(t, "root", currentUser.Username,
	"this test requires running as normal user, not %s (%s)", currentUser.Username, currentUser.Uid)
I feel there's too much nesting going on here. Also `t.FailNow()` wouldn't show the actual content of `err`. Finally, the original code was written before I realised I could do `require.NoError(...)` instead of `assert.NoError(...)` and it would immediately stop the test. This would be better: ```go // Root can always create directories, so this test would fail with a // confusing message. Instead it's better to refuse running as root at all. currentUser, err := user.Current() require.NoError(t, err) require.NotEqual(t, "0", currentUser.Uid, "this test requires running as normal user, not %s (%s)", currentUser.Username, currentUser.Uid) require.NotEqual(t, "root", currentUser.Username, "this test requires running as normal user, not %s (%s)", currentUser.Username, currentUser.Uid) ```
} else if currentUser.Username != "root" {
parentPath := filepath.Join(mf.tempdir, "deep")
testPath := filepath.Join(parentPath, "nesting")
if err := os.Mkdir(parentPath, fs.ModePerm); !assert.NoError(t, err) {
t.FailNow()
}
if err := os.Mkdir(testPath, fs.FileMode(0)); !assert.NoError(t, err) {
t.FailNow()
}
echoCtx := doTest(testPath)
result := api.PathCheckResult{}
getResponseJSON(t, echoCtx, http.StatusOK, &result)
assert.Equal(t, testPath, result.Path)
assert.False(t, result.IsUsable)
assert.Contains(t, result.Cause, "Unable to create a file")
} }
if err := os.Mkdir(testPath, fs.FileMode(0)); !assert.NoError(t, err) {
t.FailNow()
}
echoCtx := doTest(testPath)
result := api.PathCheckResult{}
getResponseJSON(t, echoCtx, http.StatusOK, &result)
assert.Equal(t, testPath, result.Path)
assert.False(t, result.IsUsable)
assert.Contains(t, result.Cause, "Unable to create a file")
} }
} }