libs/os: remove unused aliases, add test cases (#5654)

Remove unused ReadFile (unused) and
WriteFile (almost unused, alias of ioutil.WriteFile).

Add testcases for Must{Read,Write}File.

CHANGELOG_PENDING.md

@@ -18,6 +18,8 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi
 
 - Go API
 
+- [libs/os] Kill() and {Must,}{Read,Write}File() functions have been removed. (@alessio)
+
 - Blockchain Protocol
 
 ### FEATURES

config/toml.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"fmt"
 	"io/ioutil"
+	"os"
 	"path/filepath"
 	"strings"
 	"text/template"
@@ -63,7 +64,7 @@ func WriteConfigFile(configFilePath string, config *Config) {
 		panic(err)
 	}
 
-	tmos.MustWriteFile(configFilePath, buffer.Bytes(), 0644)
+	mustWriteFile(configFilePath, buffer.Bytes(), 0644)
 }
 
 // Note: any changes to the comments/variables/mapstructure
@@ -492,16 +493,22 @@ func ResetTestRootWithChainID(testName string, chainID string) *Config {
 			chainID = "tendermint_test"
 		}
 		testGenesis := fmt.Sprintf(testGenesisFmt, chainID)
-		tmos.MustWriteFile(genesisFilePath, []byte(testGenesis), 0644)
+		mustWriteFile(genesisFilePath, []byte(testGenesis), 0644)
 	}
 	// we always overwrite the priv val
-	tmos.MustWriteFile(privKeyFilePath, []byte(testPrivValidatorKey), 0644)
-	tmos.MustWriteFile(privStateFilePath, []byte(testPrivValidatorState), 0644)
+	mustWriteFile(privKeyFilePath, []byte(testPrivValidatorKey), 0644)
+	mustWriteFile(privStateFilePath, []byte(testPrivValidatorState), 0644)
 
 	config := TestConfig().SetRoot(rootDir)
 	return config
 }
 
+func mustWriteFile(filePath string, contents []byte, mode os.FileMode) {
+	if err := ioutil.WriteFile(filePath, contents, mode); err != nil {
+		tmos.Exit(fmt.Sprintf("failed to write file: %v", err))
+	}
+}
+
 var testGenesisFmt = `{
   "genesis_time": "2018-10-10T08:20:13.695936996Z",
   "chain_id": "%s",

libs/autofile/autofile_test.go

@@ -10,8 +10,6 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-
-	tmos "github.com/tendermint/tendermint/libs/os"
 )
 
 func TestSIGHUP(t *testing.T) {
@@ -27,10 +25,9 @@ func TestSIGHUP(t *testing.T) {
 	dir, err := ioutil.TempDir("", "sighup_test")
 	require.NoError(t, err)
 	t.Cleanup(func() {
-		os.RemoveAll(dir)
+		_ = os.RemoveAll(dir)
 	})
-	err = os.Chdir(dir)
-	require.NoError(t, err)
+	require.NoError(t, os.Chdir(dir))
 
 	// Create an AutoFile in the temporary directory
 	name := "sighup_test"
@@ -45,19 +42,16 @@ func TestSIGHUP(t *testing.T) {
 	require.NoError(t, err)
 
 	// Move the file over
-	err = os.Rename(name, name+"_old")
-	require.NoError(t, err)
+	require.NoError(t, os.Rename(name, name+"_old"))
 
 	// Move into a different temporary directory
 	otherDir, err := ioutil.TempDir("", "sighup_test_other")
 	require.NoError(t, err)
-	defer os.RemoveAll(otherDir)
-	err = os.Chdir(otherDir)
-	require.NoError(t, err)
+	t.Cleanup(func() { os.RemoveAll(otherDir) })
+	require.NoError(t, os.Chdir(otherDir))
 
 	// Send SIGHUP to self.
-	err = syscall.Kill(syscall.Getpid(), syscall.SIGHUP)
-	require.NoError(t, err)
+	require.NoError(t, syscall.Kill(syscall.Getpid(), syscall.SIGHUP))
 
 	// Wait a bit... signals are not handled synchronously.
 	time.Sleep(time.Millisecond * 10)
@@ -67,14 +61,13 @@ func TestSIGHUP(t *testing.T) {
 	require.NoError(t, err)
 	_, err = af.Write([]byte("Line 4\n"))
 	require.NoError(t, err)
-	err = af.Close()
-	require.NoError(t, err)
+	require.NoError(t, af.Close())
 
 	// Both files should exist
-	if body := tmos.MustReadFile(filepath.Join(dir, name+"_old")); string(body) != "Line 1\nLine 2\n" {
+	if body := mustReadFile(t, filepath.Join(dir, name+"_old")); string(body) != "Line 1\nLine 2\n" {
 		t.Errorf("unexpected body %s", body)
 	}
-	if body := tmos.MustReadFile(filepath.Join(dir, name)); string(body) != "Line 3\nLine 4\n" {
+	if body := mustReadFile(t, filepath.Join(dir, name)); string(body) != "Line 3\nLine 4\n" {
 		t.Errorf("unexpected body %s", body)
 	}
 
@@ -115,8 +108,7 @@ func TestAutoFileSize(t *testing.T) {
 	// First, create an AutoFile writing to a tempfile dir
 	f, err := ioutil.TempFile("", "sighup_test")
 	require.NoError(t, err)
-	err = f.Close()
-	require.NoError(t, err)
+	require.NoError(t, f.Close())
 
 	// Here is the actual AutoFile.
 	af, err := OpenAutoFile(f.Name())
@@ -136,14 +128,19 @@ func TestAutoFileSize(t *testing.T) {
 	require.NoError(t, err)
 
 	// 3. Not existing file
-	err = af.Close()
-	require.NoError(t, err)
-	err = os.Remove(f.Name())
-	require.NoError(t, err)
+	require.NoError(t, af.Close())
+	require.NoError(t, os.Remove(f.Name()))
 	size, err = af.Size()
 	require.EqualValues(t, 0, size, "Expected a new file to be empty")
 	require.NoError(t, err)
 
 	// Cleanup
-	_ = os.Remove(f.Name())
+	t.Cleanup(func() { os.Remove(f.Name()) })
+}
+
+func mustReadFile(t *testing.T, filePath string) []byte {
+	fileBytes, err := ioutil.ReadFile(filePath)
+	require.NoError(t, err)
+
+	return fileBytes
 }

libs/fail/fail.go

@@ -32,16 +32,9 @@ func Fail() {
 	}
 
 	if callIndex == callIndexToFail {
-		Exit()
+		fmt.Printf("*** fail-test %d ***\n", callIndex)
+		os.Exit(1)
 	}
 
 	callIndex++
 }
-
-func Exit() {
-	fmt.Printf("*** fail-test %d ***\n", callIndex)
-	os.Exit(1)
-	//	proc, _ := os.FindProcess(os.Getpid())
-	//	proc.Signal(os.Interrupt)
-	//	panic(fmt.Sprintf("*** fail-test %d ***", callIndex))
-}

libs/os/os.go

@@ -3,7 +3,6 @@ package os
 import (
 	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
 	"os/signal"
 	"syscall"
@@ -29,15 +28,6 @@ func TrapSignal(logger logger, cb func()) {
 	}()
 }
 
-// Kill the running process by sending itself SIGTERM.
-func Kill() error {
-	p, err := os.FindProcess(os.Getpid())
-	if err != nil {
-		return err
-	}
-	return p.Signal(syscall.SIGTERM)
-}
-
 func Exit(s string) {
 	fmt.Printf(s + "\n")
 	os.Exit(1)
@@ -58,30 +48,6 @@ func FileExists(filePath string) bool {
 	return !os.IsNotExist(err)
 }
 
-func ReadFile(filePath string) ([]byte, error) {
-	return ioutil.ReadFile(filePath)
-}
-
-func MustReadFile(filePath string) []byte {
-	fileBytes, err := ioutil.ReadFile(filePath)
-	if err != nil {
-		Exit(fmt.Sprintf("MustReadFile failed: %v", err))
-		return nil
-	}
-	return fileBytes
-}
-
-func WriteFile(filePath string, contents []byte, mode os.FileMode) error {
-	return ioutil.WriteFile(filePath, contents, mode)
-}
-
-func MustWriteFile(filePath string, contents []byte, mode os.FileMode) {
-	err := WriteFile(filePath, contents, mode)
-	if err != nil {
-		Exit(fmt.Sprintf("MustWriteFile failed: %v", err))
-	}
-}
-
 // CopyFile copies a file. It truncates the destination file if it exists.
 func CopyFile(src, dst string) error {
 	info, err := os.Stat(src)

libs/os/os_test.go

@@ -6,6 +6,7 @@ import (
 	"io/ioutil"
 	"os"
 	"os/exec"
+	"syscall"
 	"testing"
 	"time"
 
@@ -47,10 +48,7 @@ func TestTrapSignal(t *testing.T) {
 		return
 	}
 
-	cmd := exec.Command(os.Args[0], "-test.run="+t.Name())
-	mockStderr := bytes.NewBufferString("")
-	cmd.Env = append(os.Environ(), "TM_TRAP_SIGNAL_TEST=1")
-	cmd.Stderr = mockStderr
+	cmd, _, mockStderr := newTestProgram(t, "TM_TRAP_SIGNAL_TEST")
 
 	err := cmd.Run()
 	if err == nil {
@@ -67,7 +65,6 @@ func TestTrapSignal(t *testing.T) {
 	}
 
 	t.Fatal("this error should not be triggered")
-
 }
 
 type mockLogger struct{}
@@ -80,10 +77,26 @@ func killer() {
 	tmos.TrapSignal(logger, func() { _, _ = fmt.Fprintf(os.Stderr, "exiting") })
 	time.Sleep(1 * time.Second)
 
-	// use Kill() to test SIGTERM
-	if err := tmos.Kill(); err != nil {
+	p, err := os.FindProcess(os.Getpid())
+	if err != nil {
+		panic(err)
+	}
+
+	if err := p.Signal(syscall.SIGTERM); err != nil {
 		panic(err)
 	}
 
 	time.Sleep(1 * time.Second)
 }
+
+func newTestProgram(t *testing.T, environVar string) (cmd *exec.Cmd, stdout *bytes.Buffer, stderr *bytes.Buffer) {
+	t.Helper()
+
+	cmd = exec.Command(os.Args[0], "-test.run="+t.Name())
+	stdout, stderr = bytes.NewBufferString(""), bytes.NewBufferString("")
+	cmd.Env = append(os.Environ(), fmt.Sprintf("%s=1", environVar))
+	cmd.Stdout = stdout
+	cmd.Stderr = stderr
+
+	return
+}

p2p/conn/secret_connection_test.go

@@ -6,6 +6,7 @@ import (
 	"flag"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"log"
 	"os"
 	"path/filepath"
@@ -21,7 +22,6 @@ import (
 	"github.com/tendermint/tendermint/crypto/ed25519"
 	"github.com/tendermint/tendermint/crypto/sr25519"
 	"github.com/tendermint/tendermint/libs/async"
-	tmos "github.com/tendermint/tendermint/libs/os"
 	tmrand "github.com/tendermint/tendermint/libs/rand"
 )
 
@@ -228,8 +228,7 @@ func TestDeriveSecretsAndChallengeGolden(t *testing.T) {
 	if *update {
 		t.Logf("Updating golden test vector file %s", goldenFilepath)
 		data := createGoldenTestVectors(t)
-		err := tmos.WriteFile(goldenFilepath, []byte(data), 0644)
-		require.NoError(t, err)
+		require.NoError(t, ioutil.WriteFile(goldenFilepath, []byte(data), 0644))
 	}
 	f, err := os.Open(goldenFilepath)
 	if err != nil {

proxy/multi_app_conn.go

@@ -2,10 +2,11 @@ package proxy
 
 import (
 	"fmt"
+	"os"
+	"syscall"
 
 	abcicli "github.com/tendermint/tendermint/abci/client"
 	tmlog "github.com/tendermint/tendermint/libs/log"
-	tmos "github.com/tendermint/tendermint/libs/os"
 	"github.com/tendermint/tendermint/libs/service"
 )
 
@@ -129,8 +130,7 @@ func (app *multiAppConn) killTMOnClientError() {
 		logger.Error(
 			fmt.Sprintf("%s connection terminated. Did the application crash? Please restart tendermint", conn),
 			"err", err)
-		killErr := tmos.Kill()
-		if killErr != nil {
+		if killErr := kill(); killErr != nil {
 			logger.Error("Failed to kill this process - please do so manually", "err", killErr)
 		}
 	}
@@ -189,3 +189,12 @@ func (app *multiAppConn) abciClientFor(conn string) (abcicli.Client, error) {
 	}
 	return c, nil
 }
+
+func kill() error {
+	p, err := os.FindProcess(os.Getpid())
+	if err != nil {
+		return err
+	}
+
+	return p.Signal(syscall.SIGTERM)
+}

types/genesis.go

@@ -11,7 +11,6 @@ import (
 	"github.com/tendermint/tendermint/crypto"
 	tmbytes "github.com/tendermint/tendermint/libs/bytes"
 	tmjson "github.com/tendermint/tendermint/libs/json"
-	tmos "github.com/tendermint/tendermint/libs/os"
 	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 	tmtime "github.com/tendermint/tendermint/types/time"
 )
@@ -52,7 +51,8 @@ func (genDoc *GenesisDoc) SaveAs(file string) error {
 	if err != nil {
 		return err
 	}
-	return tmos.WriteFile(file, genDocBytes, 0644)
+
+	return ioutil.WriteFile(file, genDocBytes, 0644) // nolint:gosec
 }
 
 // ValidatorHash returns the hash of the validator set contained in the GenesisDoc