fix up tests; improved error handling throughout

Author: symonk

.golangci.yml

@@ -6,7 +6,6 @@ linters:
   enable:
     - errcheck
     - goimports
-    - golint
     - govet
     - staticcheck
 

profile.go

@@ -29,7 +29,7 @@ const (
 
 // FinalizerFunc is a function that is invokved during the teardown period
 // of the profiling instance.
-type FinalizerFunc func()
+type FinalizerFunc func() error
 
 // CallbackFunc is a function that can be supplied with the
 // WithCallback option to be executed when the profiling instance
@@ -90,7 +90,9 @@ func (p *Profiler) Stop() {
 	if !atomic.CompareAndSwapUint32(&profilingActive, 1, 0) {
 		die("profiler instance was not started")
 	}
-	p.finalizer()
+	if err := p.finalizer(); err != nil {
+		die(err.Error())
+	}
 	if p.callback != nil {
 		p.callback(p)
 	}

profile_test.go

@@ -21,7 +21,11 @@ func TestProfilesEnabledExpectedOutput(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer os.RemoveAll(storage)
+	defer func() {
+		if err := os.RemoveAll(storage); err != nil {
+			t.Error("problem removing file", err)
+		}
+	}()
 	tests := map[string]struct {
 		source string
 		checks []CheckFunc
@@ -102,25 +106,25 @@ func createTempTestFile(t *testing.T, source string) (string, func()) {
 go 1.23.2
 require github.com/symonk/profiler v0.0.0-20241021143805-788e1dbe92a9
 `
-	os.WriteFile(mod, []byte(contents), 0644)
+	if err := os.WriteFile(mod, []byte(contents), 0644); err != nil {
+		t.Error("failed to write file", err)
+	}
 
 	// create the appropriate mod file etc
-	return main, func() { defer os.RemoveAll(dir) }
+	return main, func() {
+		defer func() {
+			if err := os.RemoveAll(dir); err != nil {
+				t.Error("failed to remove dir", err)
+			}
+		}()
+	}
 }
 
 // Check function implementations for asserting against the responses
 func exitedZero(t *testing.T, _, _ string, code int) {
 	assert.Zero(t, code)
 }
 
-// patternMatchLines checks that the lines in stdout matched
-func stdOutOutMatchLines(patterns ...string) CheckFunc {
-	return func(t *testing.T, stdout, stderr string, exit int) {
-		assert.NotEmpty(t, stdout)
-		patternMatchLines(t, stdout, patterns...)
-	}
-}
-
 // patternMatchLines checks that the lines in stderr matched
 func stdErrOutMatchLines(patterns ...string) CheckFunc {
 	return func(t *testing.T, stdout, stderr string, exit int) {
@@ -156,10 +160,6 @@ func patternMatchLines(t *testing.T, input string, patterns ...string) bool {
 	return false
 }
 
-func emptyStdErr(t *testing.T, _, stderr string, _ int) {
-	assert.Empty(t, stderr)
-}
-
 func emptyStdOut(t *testing.T, stdout, _ string, _ int) {
 	assert.Empty(t, stdout)
 }

strategy.go

@@ -33,85 +33,91 @@ func cpuStrategyFn(p *Profiler) (FinalizerFunc, error) {
 	if err := pprof.StartCPUProfile(p.profileFile); err != nil {
 		return nil, err
 	}
-	return func() {
+	return func() (err error) {
+		defer func() { err = p.profileFile.Close() }()
 		pprof.StopCPUProfile()
-		p.profileFile.Close()
+		return nil
 	}, nil
 }
 
 func heapStrategyFn(p *Profiler) (FinalizerFunc, error) {
 	rate := runtime.MemProfileRate
 	p.SetProfileFile(MemoryFileName)
 	runtime.MemProfileRate = p.memoryProfileRate
-	return func() {
+	return func() (err error) {
+		defer func() { runtime.MemProfileRate = rate }()
+		defer func() { err = p.profileFile.Close() }()
 		_ = pprof.Lookup(heapProfileName).WriteTo(p.profileFile, 0)
 		runtime.GC()
-		p.profileFile.Close()
-		runtime.MemProfileRate = rate
+		return nil
 	}, nil
 }
 
 func allocStrategyFn(p *Profiler) (FinalizerFunc, error) {
 	rate := runtime.MemProfileRate
 	p.SetProfileFile(MemoryFileName)
 	runtime.MemProfileRate = p.memoryProfileRate
-	return func() {
+	return func() (err error) {
+		defer func() { runtime.MemProfileRate = rate }()
+		defer func() { err = p.profileFile.Close() }()
 		_ = pprof.Lookup(allocProfileName).WriteTo(p.profileFile, 0)
 		runtime.GC()
-		p.profileFile.Close()
-		runtime.MemProfileRate = rate
+		return nil
 	}, nil
 }
 
 func mutexStrategyFn(p *Profiler) (FinalizerFunc, error) {
 	p.SetProfileFile(MutexFileName)
 	_ = pprof.Lookup("mutex").WriteTo(p.profileFile, 0)
-	return func() {
-		p.profileFile.Close()
+	return func() error {
+		return p.profileFile.Close()
 	}, nil
 }
 
 func blockStrategyFn(p *Profiler) (FinalizerFunc, error) {
 	p.SetProfileFile(BlockFileName)
 	// for now, we do not allow customising the runtime.SetBlockProfileRate
 	// if it is useful in future, change is welcome here.
-	return func() {
+	return func() error {
+		defer runtime.SetBlockProfileRate(0)
 		_ = pprof.Lookup("block").WriteTo(p.profileFile, 0)
-		p.profileFile.Close()
-		runtime.SetBlockProfileRate(0)
+		return p.profileFile.Close()
 	}, nil
 }
 
 func goroutineStrategyFn(p *Profiler) (FinalizerFunc, error) {
 	p.SetProfileFile(GoroutineFileName)
 	_ = pprof.Lookup("goroutine").WriteTo(p.profileFile, 0)
-	return func() {
-		p.profileFile.Close()
+	return func() error {
+		return p.profileFile.Close()
 	}, nil
 }
 
 func threadCreateStrategyFn(p *Profiler) (FinalizerFunc, error) {
 	p.SetProfileFile(ThreadCreateFileName)
-	return func() {
+	return func() (err error) {
+		defer func() { err = p.profileFile.Close() }()
 		_ = pprof.Lookup("threadcreate").WriteTo(p.profileFile, 0)
-		p.profileFile.Close()
+		return nil
 	}, nil
 }
 
 func traceStrategyFn(p *Profiler) (FinalizerFunc, error) {
 	p.SetProfileFile(TraceFileName)
-	trace.Start(p.profileFile)
-	return func() {
-		p.profileFile.Close()
+	if err := trace.Start(p.profileFile); err != nil {
+		return nil, err
+	}
+	return func() error {
 		trace.Stop()
+		return nil
 	}, nil
 }
 
 func clockStrategyFn(p *Profiler) (FinalizerFunc, error) {
 	p.SetProfileFile(ClockFileName)
 	teardown := fgprof.Start(p.profileFile, fgprof.FormatPprof)
-	return func() {
-		teardown()
-		p.profileFile.Close()
+	return func() (err error) {
+		defer func() { err = p.profileFile.Close() }()
+		return teardown()
 	}, nil
 }