Fix linter warnings

Fix most linter warnings reported by 'staticcheck'. This doesn't fix all
of them, some unused functions are still there, and some generated code
also still triggers some warnings. Most issues are fixed, though.

No functional changes, except for the captialisation of some error
messages.
This commit is contained in:
Sybren A. Stüvel 2024-12-01 14:46:46 +01:00
parent 167dd027c1
commit a9bec98fcd
17 changed files with 36 additions and 51 deletions

View File

@ -16,6 +16,7 @@ type DummyShaman struct{}
var _ api_impl.Shaman = (*DummyShaman)(nil) var _ api_impl.Shaman = (*DummyShaman)(nil)
//lint:ignore ST1005 Shaman is a name that's always capitalised, so also in errors.
var ErrDummyShaman = errors.New("Shaman storage component is inactive, configure Flamenco first") var ErrDummyShaman = errors.New("Shaman storage component is inactive, configure Flamenco first")
func (ds *DummyShaman) IsEnabled() bool { func (ds *DummyShaman) IsEnabled() bool {

View File

@ -5,7 +5,6 @@ package task_logs
import ( import (
"errors" "errors"
"io/fs" "io/fs"
"io/ioutil"
"os" "os"
"path/filepath" "path/filepath"
"testing" "testing"
@ -16,7 +15,7 @@ import (
) )
func setUpTest(t *testing.T) string { func setUpTest(t *testing.T) string {
temppath, err := ioutil.TempDir("", "testlogs") temppath, err := os.MkdirTemp("", "testlogs")
require.NoError(t, err) require.NoError(t, err)
return temppath return temppath
} }
@ -78,12 +77,12 @@ func TestMultipleFilesWithHoles(t *testing.T) {
defer tearDownTest(temppath) defer tearDownTest(temppath)
filepath := filepath.Join(temppath, "existing.txt") filepath := filepath.Join(temppath, "existing.txt")
require.NoError(t, ioutil.WriteFile(filepath, []byte("thefile"), 0666)) require.NoError(t, os.WriteFile(filepath, []byte("thefile"), 0666))
require.NoError(t, ioutil.WriteFile(filepath+".1", []byte("file .1"), 0666)) require.NoError(t, os.WriteFile(filepath+".1", []byte("file .1"), 0666))
require.NoError(t, ioutil.WriteFile(filepath+".2", []byte("file .2"), 0666)) require.NoError(t, os.WriteFile(filepath+".2", []byte("file .2"), 0666))
require.NoError(t, ioutil.WriteFile(filepath+".3", []byte("file .3"), 0666)) require.NoError(t, os.WriteFile(filepath+".3", []byte("file .3"), 0666))
require.NoError(t, ioutil.WriteFile(filepath+".5", []byte("file .5"), 0666)) require.NoError(t, os.WriteFile(filepath+".5", []byte("file .5"), 0666))
require.NoError(t, ioutil.WriteFile(filepath+".7", []byte("file .7"), 0666)) require.NoError(t, os.WriteFile(filepath+".7", []byte("file .7"), 0666))
err := rotateLogFile(zerolog.Nop(), filepath) err := rotateLogFile(zerolog.Nop(), filepath)
@ -100,7 +99,7 @@ func TestMultipleFilesWithHoles(t *testing.T) {
assert.False(t, fileExists(filepath+".9")) assert.False(t, fileExists(filepath+".9"))
read := func(filename string) string { read := func(filename string) string {
content, err := ioutil.ReadFile(filename) content, err := os.ReadFile(filename)
require.NoError(t, err) require.NoError(t, err)
return string(content) return string(content)
} }

View File

@ -185,7 +185,7 @@ func (s *Storage) Tail(jobID, taskID string) (string, error) {
return "", fmt.Errorf("unable to open log file for reading: %w", err) return "", fmt.Errorf("unable to open log file for reading: %w", err)
} }
fileSize, err := file.Seek(0, os.SEEK_END) fileSize, err := file.Seek(0, io.SeekEnd)
if err != nil { if err != nil {
return "", fmt.Errorf("unable to seek to end of log file: %w", err) return "", fmt.Errorf("unable to seek to end of log file: %w", err)
} }
@ -197,14 +197,14 @@ func (s *Storage) Tail(jobID, taskID string) (string, error) {
) )
if fileSize <= tailSize { if fileSize <= tailSize {
// The file is small, just read all of it. // The file is small, just read all of it.
_, err = file.Seek(0, os.SEEK_SET) _, err = file.Seek(0, io.SeekStart)
if err != nil { if err != nil {
return "", fmt.Errorf("unable to seek to start of log file: %w", err) return "", fmt.Errorf("unable to seek to start of log file: %w", err)
} }
buffer, err = io.ReadAll(file) buffer, err = io.ReadAll(file)
} else { } else {
// Read the last 'tailSize' number of bytes. // Read the last 'tailSize' number of bytes.
_, err = file.Seek(-tailSize, os.SEEK_END) _, err = file.Seek(-tailSize, io.SeekEnd)
if err != nil { if err != nil {
return "", fmt.Errorf("unable to seek in log file: %w", err) return "", fmt.Errorf("unable to seek in log file: %w", err)
} }

View File

@ -6,7 +6,6 @@ import (
"errors" "errors"
"fmt" "fmt"
"io/fs" "io/fs"
"io/ioutil"
"os" "os"
"path/filepath" "path/filepath"
"strings" "strings"
@ -43,7 +42,7 @@ func TestLogWriting(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
filename := filepath.Join(jobDir, "task-20ff9d06-53ec-4019-9e2e-1774f05f170a.txt") filename := filepath.Join(jobDir, "task-20ff9d06-53ec-4019-9e2e-1774f05f170a.txt")
contents, err := ioutil.ReadFile(filename) contents, err := os.ReadFile(filename)
require.NoError(t, err, "the log file should exist") require.NoError(t, err, "the log file should exist")
assert.Equal(t, "Ovo je priča\nIma dvije linije\n", string(contents)) assert.Equal(t, "Ovo je priča\nIma dvije linije\n", string(contents))
} }
@ -67,7 +66,7 @@ func TestLogRotation(t *testing.T) {
filename := filepath.Join(jobDir, "task-20ff9d06-53ec-4019-9e2e-1774f05f170a.txt") filename := filepath.Join(jobDir, "task-20ff9d06-53ec-4019-9e2e-1774f05f170a.txt")
rotatedFilename := filename + ".1" rotatedFilename := filename + ".1"
contents, err := ioutil.ReadFile(rotatedFilename) contents, err := os.ReadFile(rotatedFilename)
require.NoError(t, err, "the rotated log file should exist") require.NoError(t, err, "the rotated log file should exist")
assert.Equal(t, "Ovo je priča\n", string(contents)) assert.Equal(t, "Ovo je priča\n", string(contents))
@ -216,7 +215,7 @@ type TaskLogsMocks struct {
func taskLogsTestFixtures(t *testing.T) (*Storage, func(), *TaskLogsMocks) { func taskLogsTestFixtures(t *testing.T) (*Storage, func(), *TaskLogsMocks) {
mockCtrl := gomock.NewController(t) mockCtrl := gomock.NewController(t)
temppath, err := ioutil.TempDir("", "testlogs") temppath, err := os.MkdirTemp("", "testlogs")
require.NoError(t, err) require.NoError(t, err)
mocks := &TaskLogsMocks{ mocks := &TaskLogsMocks{

View File

@ -5,14 +5,6 @@ package task_state_machine
import "projects.blender.org/studio/flamenco/pkg/api" import "projects.blender.org/studio/flamenco/pkg/api"
var ( var (
// Task statuses that always get requeued when the job is requeueing.
nonCompletedStatuses = []api.TaskStatus{
api.TaskStatusCanceled,
api.TaskStatusFailed,
api.TaskStatusPaused,
api.TaskStatusSoftFailed,
}
// Workers are allowed to keep running tasks when they are in this status. // Workers are allowed to keep running tasks when they are in this status.
// 'queued', 'claimed-by-manager', and 'soft-failed' aren't considered runnable, // 'queued', 'claimed-by-manager', and 'soft-failed' aren't considered runnable,
// as those statuses indicate the task wasn't assigned to a Worker by the scheduler. // as those statuses indicate the task wasn't assigned to a Worker by the scheduler.

View File

@ -79,6 +79,7 @@ func stressByRequestingTask(ctx context.Context, client worker.FlamencoClient) {
} }
} }
//lint:ignore U1000 stressBySendingTaskUpdate is currently unused, but someone may find it useful for different kinds of stess testing.
func stressBySendingTaskUpdate(ctx context.Context, client worker.FlamencoClient, task *api.AssignedTask) { func stressBySendingTaskUpdate(ctx context.Context, client worker.FlamencoClient, task *api.AssignedTask) {
logLine := "This is a log-line for stress testing. It will be repeated more than once.\n" logLine := "This is a log-line for stress testing. It will be repeated more than once.\n"
logToSend := strings.Repeat(logLine, 5) logToSend := strings.Repeat(logLine, 5)

View File

@ -46,7 +46,7 @@ func AutodiscoverManager(ctx context.Context) (string, error) {
logger := log.Logger logger := log.Logger
if deadline, ok := ctx.Deadline(); ok { if deadline, ok := ctx.Deadline(); ok {
timeout := deadline.Sub(time.Now()).Round(1 * time.Second) timeout := time.Until(deadline).Round(1 * time.Second)
logger = logger.With().Str("timeout", timeout.String()).Logger() logger = logger.With().Str("timeout", timeout.String()).Logger()
} }
logger.Info().Msg("auto-discovering Manager via UPnP/SSDP") logger.Info().Msg("auto-discovering Manager via UPnP/SSDP")

View File

@ -101,7 +101,6 @@ readloop:
// Prepend any leftovers from the previous line to the received bytes. // Prepend any leftovers from the previous line to the received bytes.
if len(leftovers) > 0 { if len(leftovers) > 0 {
lineBytes = append(leftovers, lineBytes...) lineBytes = append(leftovers, lineBytes...)
leftovers = []byte{}
} }
// Make sure long lines are broken on character boundaries. // Make sure long lines are broken on character boundaries.
lineBytes, leftovers = splitOnCharacterBoundary(lineBytes) lineBytes, leftovers = splitOnCharacterBoundary(lineBytes)

View File

@ -133,7 +133,7 @@ func (ce *CommandExecutor) cmdCopyFile(ctx context.Context, logger zerolog.Logge
return err return err
} }
err, logMsg := fileCopy(src, dest) logMsg, err := fileCopy(src, dest)
return ce.errorLogProcess(ctx, logger, cmd, taskID, err, logMsg) return ce.errorLogProcess(ctx, logger, cmd, taskID, err, logMsg)
} }
@ -171,48 +171,48 @@ func (ce *CommandExecutor) moveAndLog(ctx context.Context, taskID, cmdName, src,
return nil return nil
} }
func fileCopy(src, dest string) (error, string) { func fileCopy(src, dest string) (string, error) {
src_file, err := os.Open(src) src_file, err := os.Open(src)
if err != nil { if err != nil {
msg := fmt.Sprintf("failed to open source file %q: %v", src, err) msg := fmt.Sprintf("failed to open source file %q: %v", src, err)
return err, msg return msg, err
} }
defer src_file.Close() defer src_file.Close()
src_file_stat, err := src_file.Stat() src_file_stat, err := src_file.Stat()
if err != nil { if err != nil {
msg := fmt.Sprintf("failed to stat source file %q: %v", src, err) msg := fmt.Sprintf("failed to stat source file %q: %v", src, err)
return err, msg return msg, err
} }
if !src_file_stat.Mode().IsRegular() { if !src_file_stat.Mode().IsRegular() {
err := &os.PathError{Op: "stat", Path: src, Err: errors.New("Not a regular file")} err := &os.PathError{Op: "stat", Path: src, Err: errors.New("not a regular file")}
msg := fmt.Sprintf("invalid source file %q: %v", src, err) msg := fmt.Sprintf("invalid source file %q: %v", src, err)
return err, msg return msg, err
} }
dest_dirpath := filepath.Dir(dest) dest_dirpath := filepath.Dir(dest)
if !fileExists(dest_dirpath) { if !fileExists(dest_dirpath) {
if err := os.MkdirAll(dest_dirpath, 0750); err != nil { if err := os.MkdirAll(dest_dirpath, 0750); err != nil {
msg := fmt.Sprintf("failed to create directories %q: %v", dest_dirpath, err) msg := fmt.Sprintf("failed to create directories %q: %v", dest_dirpath, err)
return err, msg return msg, err
} }
} }
dest_file, err := os.Create(dest) dest_file, err := os.Create(dest)
if err != nil { if err != nil {
msg := fmt.Sprintf("failed to create destination file %q: %v", dest, err) msg := fmt.Sprintf("failed to create destination file %q: %v", dest, err)
return err, msg return msg, err
} }
defer dest_file.Close() defer dest_file.Close()
if _, err := io.Copy(dest_file, src_file); err != nil { if _, err := io.Copy(dest_file, src_file); err != nil {
msg := fmt.Sprintf("failed to copy %q to %q: %v", src, dest, err) msg := fmt.Sprintf("failed to copy %q to %q: %v", src, dest, err)
return err, msg return msg, err
} }
msg := fmt.Sprintf("copied %q to %q", src, dest) msg := fmt.Sprintf("copied %q to %q", src, dest)
return nil, msg return msg, nil
} }
func fileExists(filename string) bool { func fileExists(filename string) bool {

View File

@ -163,7 +163,7 @@ func TestTimestampedPathFile(t *testing.T) {
fileCreateEmpty("somefile.txt") fileCreateEmpty("somefile.txt")
if err := os.Chtimes("somefile.txt", mtime, mtime); err != nil { if err := os.Chtimes("somefile.txt", mtime, mtime); err != nil {
t.Fatalf(err.Error()) t.Fatal(err.Error())
} }
newpath, err := timestampedPath("somefile.txt") newpath, err := timestampedPath("somefile.txt")
@ -365,7 +365,7 @@ func TestCmdCopyFileSourceIsDir(t *testing.T) {
fmt.Sprintf("copy-file: copying %q to %q", f.absolute_src_path, f.absolute_dest_path)) fmt.Sprintf("copy-file: copying %q to %q", f.absolute_src_path, f.absolute_dest_path))
f.mocks.listener.EXPECT().LogProduced(gomock.Any(), taskID, f.mocks.listener.EXPECT().LogProduced(gomock.Any(), taskID,
fmt.Sprintf("copy-file: invalid source file %q: stat %s: Not a regular file", fmt.Sprintf("copy-file: invalid source file %q: stat %s: not a regular file",
f.absolute_src_path, f.absolute_src_path)) f.absolute_src_path, f.absolute_src_path))
assert.Error(t, f.run()) assert.Error(t, f.run())

View File

@ -185,7 +185,6 @@ checkloop:
break checkloop break checkloop
case <-time.After(mayKeepRunningPeriod): case <-time.After(mayKeepRunningPeriod):
// Time to do another check. // Time to do another check.
break
} }
mkr := w.mayIKeepRunning(taskCtx, task.Uuid) mkr := w.mayIKeepRunning(taskCtx, task.Uuid)

View File

@ -73,7 +73,7 @@ func (err ErrInvalidCheckoutPath) Error() string {
// Errors returned by the Checkout Manager. // Errors returned by the Checkout Manager.
var ( var (
ErrCheckoutAlreadyExists = errors.New("A checkout with this ID already exists") ErrCheckoutAlreadyExists = errors.New("a checkout with this ID already exists")
) )
// NewManager creates and returns a new Checkout Manager. // NewManager creates and returns a new Checkout Manager.

View File

@ -26,7 +26,7 @@ import (
"bytes" "bytes"
"context" "context"
"io" "io"
"io/ioutil" "os"
"testing" "testing"
"projects.blender.org/studio/flamenco/pkg/shaman/config" "projects.blender.org/studio/flamenco/pkg/shaman/config"
@ -86,7 +86,7 @@ func TestStoreFile(t *testing.T) {
assert.Equal(t, filestore.StatusStored, status) assert.Equal(t, filestore.StatusStored, status)
assert.FileExists(t, path) assert.FileExists(t, path)
savedContent, err := ioutil.ReadFile(path) savedContent, err := os.ReadFile(path)
require.NoError(t, err) require.NoError(t, err)
assert.EqualValues(t, payload, savedContent, "The file should be saved uncompressed") assert.EqualValues(t, payload, savedContent, "The file should be saved uncompressed")
} }

View File

@ -47,9 +47,7 @@ func (fs *FileServer) receiveListenerFor(checksum string, filesize int64) chan s
go func() { go func() {
// Wait until the channel closes. // Wait until the channel closes.
select { <-channel
case <-channel:
}
fs.receiverMutex.Lock() fs.receiverMutex.Lock()
defer fs.receiverMutex.Unlock() defer fs.receiverMutex.Unlock()

View File

@ -23,7 +23,6 @@
package filestore package filestore
import ( import (
"io/ioutil"
"os" "os"
"path/filepath" "path/filepath"
"testing" "testing"
@ -115,7 +114,7 @@ func TestOpenForUpload(t *testing.T) {
assert.Equal(t, file.Name(), foundPath) assert.Equal(t, file.Name(), foundPath)
assert.Equal(t, StatusUploading, status) assert.Equal(t, StatusUploading, status)
readContents, err := ioutil.ReadFile(foundPath) readContents, err := os.ReadFile(foundPath)
require.NoError(t, err) require.NoError(t, err)
assert.EqualValues(t, contents, readContents) assert.EqualValues(t, contents, readContents)
} }

View File

@ -25,7 +25,6 @@ package filestore
import ( import (
"fmt" "fmt"
"io" "io"
"io/ioutil"
"os" "os"
"path/filepath" "path/filepath"
"runtime" "runtime"
@ -36,7 +35,7 @@ import (
// CreateTestStore returns a Store that can be used for unit testing. // CreateTestStore returns a Store that can be used for unit testing.
func CreateTestStore() *Store { func CreateTestStore() *Store {
tempDir, err := ioutil.TempDir("", "shaman-filestore-test-") tempDir, err := os.MkdirTemp("", "shaman-filestore-test-")
if err != nil { if err != nil {
panic(err) panic(err)
} }

View File

@ -23,7 +23,6 @@
package touch package touch
import ( import (
"io/ioutil"
"os" "os"
"testing" "testing"
"time" "time"
@ -36,7 +35,7 @@ func TestTouch(t *testing.T) {
testPath := "_touch_test.txt" testPath := "_touch_test.txt"
// Create a file // Create a file
assert.Nil(t, ioutil.WriteFile(testPath, []byte("just a test"), 0644)) assert.Nil(t, os.WriteFile(testPath, []byte("just a test"), 0644))
defer os.Remove(testPath) defer os.Remove(testPath)
// Make it old // Make it old