test(infra): improved support for in-process testing (#2169)

* feat(infra): improved support for in-process testing

* support for killing of a running server using simulated Ctrl-C
* support for overriding os.Stdin
* migrated many tests from the exe runner to in-process runner

* added required indirection when defining Envar() so we can later override it in tests

* refactored CLI runners by moving environment overrides to CLITestEnv

.github/workflows/race-detector.yml

@@ -21,6 +21,6 @@ jobs:
       with:
         fetch-depth: 0
     - name: Unit Tests
-      run: make -j2 test UNIT_TEST_RACE_FLAGS=-race
+      run: make -j2 test UNIT_TEST_RACE_FLAGS=-race UNIT_TESTS_TIMEOUT=1200s
     - name: Integration Tests
       run: make -j2 ci-integration-tests INTEGRATION_TEST_RACE_FLAGS=-race

.golangci.yml

@@ -26,6 +26,7 @@ linters-settings:
     - time.Now # do not use outside of 'clock' and 'timetrack' packages use clock.Now or timetrack.StartTimer
     - time.Since # use timetrack.Timer.Elapsed()
     - time.Until # never use this
+    - Envar\(\" # do not use envar literals, always wrap with EnvName()
   funlen:
     lines: 100
     statements: 60

Makefile

@@ -42,7 +42,7 @@ GOTESTSUM_FLAGS=--format=$(GOTESTSUM_FORMAT) --no-summary=skipped
 GO_TEST?=$(gotestsum) $(GOTESTSUM_FLAGS) --
 
 LINTER_DEADLINE=600s
-UNIT_TESTS_TIMEOUT=300s
+UNIT_TESTS_TIMEOUT=600s
 
 ifeq ($(GOARCH),amd64)
 PARALLEL=8
@@ -233,11 +233,13 @@ dev-deps:
 	GO111MODULE=off go get -u github.com/sqs/goreturns
 
 test-with-coverage: export KOPIA_COVERAGE_TEST=1
-test-with-coverage: $(gotestsum)
+test-with-coverage: export TESTING_ACTION_EXE ?= $(TESTING_ACTION_EXE)
+test-with-coverage: $(gotestsum) $(TESTING_ACTION_EXE)
 	$(GO_TEST) $(UNIT_TEST_RACE_FLAGS) -tags testing -count=$(REPEAT_TEST) -short -covermode=atomic -coverprofile=coverage.txt --coverpkg $(COVERAGE_PACKAGES) -timeout 300s ./...
 
 test: GOTESTSUM_FLAGS=--format=$(GOTESTSUM_FORMAT) --no-summary=skipped --jsonfile=.tmp.unit-tests.json
-test: $(gotestsum)
+test: export TESTING_ACTION_EXE ?= $(TESTING_ACTION_EXE)
+test: $(gotestsum) $(TESTING_ACTION_EXE)
 	$(GO_TEST) $(UNIT_TEST_RACE_FLAGS) -tags testing -count=$(REPEAT_TEST) -timeout $(UNIT_TESTS_TIMEOUT) ./...
 	-$(gotestsum) tool slowest --jsonfile .tmp.unit-tests.json  --threshold 1000ms
 

cli/app.go

@@ -87,6 +87,9 @@ type appServices interface {
 	getProgress() *cliProgress
 	stdout() io.Writer
 	Stderr() io.Writer
+	stdin() io.Reader
+	onCtrlC(callback func())
+	EnvName(s string) string
 }
 
 type advancedAppServices interface {
@@ -150,17 +153,24 @@ type App struct {
 	logs        commandLogs
 
 	// testability hooks
-	osExit        func(int) // allows replacing os.Exit() with custom code
-	stdoutWriter  io.Writer
-	stderrWriter  io.Writer
-	rootctx       context.Context // nolint:containedctx
-	loggerFactory logging.LoggerFactory
+	osExit         func(int) // allows replacing os.Exit() with custom code
+	stdinReader    io.Reader
+	stdoutWriter   io.Writer
+	stderrWriter   io.Writer
+	rootctx        context.Context // nolint:containedctx
+	loggerFactory  logging.LoggerFactory
+	simulatedCtrlC chan bool
+	envNamePrefix  string
 }
 
 func (c *App) getProgress() *cliProgress {
 	return c.progress
 }
 
+func (c *App) stdin() io.Reader {
+	return c.stdinReader
+}
+
 func (c *App) stdout() io.Writer {
 	return c.stdoutWriter
 }
@@ -222,21 +232,21 @@ func (c *App) setup(app *kingpin.Application) {
 	app.Flag("auto-maintenance", "Automatic maintenance").Default("true").Hidden().BoolVar(&c.enableAutomaticMaintenance)
 
 	// hidden flags to control auto-update behavior.
-	app.Flag("initial-update-check-delay", "Initial delay before first time update check").Default("24h").Hidden().Envar("KOPIA_INITIAL_UPDATE_CHECK_DELAY").DurationVar(&c.initialUpdateCheckDelay)
-	app.Flag("update-check-interval", "Interval between update checks").Default("168h").Hidden().Envar("KOPIA_UPDATE_CHECK_INTERVAL").DurationVar(&c.updateCheckInterval)
-	app.Flag("update-available-notify-interval", "Interval between update notifications").Default("1h").Hidden().Envar("KOPIA_UPDATE_NOTIFY_INTERVAL").DurationVar(&c.updateAvailableNotifyInterval)
-	app.Flag("config-file", "Specify the config file to use").Default("repository.config").Envar("KOPIA_CONFIG_PATH").StringVar(&c.configPath)
+	app.Flag("initial-update-check-delay", "Initial delay before first time update check").Default("24h").Hidden().Envar(c.EnvName("KOPIA_INITIAL_UPDATE_CHECK_DELAY")).DurationVar(&c.initialUpdateCheckDelay)
+	app.Flag("update-check-interval", "Interval between update checks").Default("168h").Hidden().Envar(c.EnvName("KOPIA_UPDATE_CHECK_INTERVAL")).DurationVar(&c.updateCheckInterval)
+	app.Flag("update-available-notify-interval", "Interval between update notifications").Default("1h").Hidden().Envar(c.EnvName("KOPIA_UPDATE_NOTIFY_INTERVAL")).DurationVar(&c.updateAvailableNotifyInterval)
+	app.Flag("config-file", "Specify the config file to use").Default("repository.config").Envar(c.EnvName("KOPIA_CONFIG_PATH")).StringVar(&c.configPath)
 	app.Flag("trace-storage", "Enables tracing of storage operations.").Default("true").Hidden().BoolVar(&c.traceStorage)
 	app.Flag("timezone", "Format time according to specified time zone (local, utc, original or time zone name)").Hidden().StringVar(&timeZone)
-	app.Flag("password", "Repository password.").Envar("KOPIA_PASSWORD").Short('p').StringVar(&c.password)
-	app.Flag("persist-credentials", "Persist credentials").Default("true").Envar("KOPIA_PERSIST_CREDENTIALS_ON_CONNECT").BoolVar(&c.persistCredentials)
-	app.Flag("disable-internal-log", "Disable internal log").Hidden().Envar("KOPIA_DISABLE_INTERNAL_LOG").BoolVar(&c.disableInternalLog)
-	app.Flag("advanced-commands", "Enable advanced (and potentially dangerous) commands.").Hidden().Envar("KOPIA_ADVANCED_COMMANDS").StringVar(&c.AdvancedCommands)
-	app.Flag("track-releasable", "Enable tracking of releasable resources.").Hidden().Envar("KOPIA_TRACK_RELEASABLE").StringsVar(&c.trackReleasable)
+	app.Flag("password", "Repository password.").Envar(c.EnvName("KOPIA_PASSWORD")).Short('p').StringVar(&c.password)
+	app.Flag("persist-credentials", "Persist credentials").Default("true").Envar(c.EnvName("KOPIA_PERSIST_CREDENTIALS_ON_CONNECT")).BoolVar(&c.persistCredentials)
+	app.Flag("disable-internal-log", "Disable internal log").Hidden().Envar(c.EnvName("KOPIA_DISABLE_INTERNAL_LOG")).BoolVar(&c.disableInternalLog)
+	app.Flag("advanced-commands", "Enable advanced (and potentially dangerous) commands.").Hidden().Envar(c.EnvName("KOPIA_ADVANCED_COMMANDS")).StringVar(&c.AdvancedCommands)
+	app.Flag("track-releasable", "Enable tracking of releasable resources.").Hidden().Envar(c.EnvName("KOPIA_TRACK_RELEASABLE")).StringsVar(&c.trackReleasable)
 
-	c.observability.setup(app)
+	c.observability.setup(c, app)
 
-	c.setupOSSpecificKeychainFlags(app)
+	c.setupOSSpecificKeychainFlags(c, app)
 
 	_ = app.Flag("caching", "Enables caching of objects (disable with --no-caching)").Default("true").Hidden().Action(
 		deprecatedFlag(c.stderrWriter, "The '--caching' flag is deprecated and has no effect, use 'kopia cache set' instead."),
@@ -298,10 +308,21 @@ func NewApp() *App {
 		osExit:       os.Exit,
 		stdoutWriter: colorable.NewColorableStdout(),
 		stderrWriter: colorable.NewColorableStderr(),
+		stdinReader:  os.Stdin,
 		rootctx:      context.Background(),
 	}
 }
 
+// SetEnvNamePrefixForTesting sets the name prefix to be used for all environment variable names for testing.
+func (c *App) SetEnvNamePrefixForTesting(prefix string) {
+	c.envNamePrefix = prefix
+}
+
+// EnvName overrides the provided environment variable name for testability.
+func (c *App) EnvName(n string) string {
+	return c.envNamePrefix + n
+}
+
 // Attach attaches the CLI parser to the application.
 func (c *App) Attach(app *kingpin.Application) {
 	c.setup(app)

cli/command_diff.go

@@ -26,7 +26,7 @@ func (c *commandDiff) setup(svc appServices, parent commandParent) {
 	cmd.Arg("object-path1", "First object/path").Required().StringVar(&c.diffFirstObjectPath)
 	cmd.Arg("object-path2", "Second object/path").Required().StringVar(&c.diffSecondObjectPath)
 	cmd.Flag("files", "Compare files by launching diff command for all pairs of (old,new)").Short('f').BoolVar(&c.diffCompareFiles)
-	cmd.Flag("diff-command", "Displays differences between two repository objects (files or directories)").Default(defaultDiffCommand()).Envar("KOPIA_DIFF").StringVar(&c.diffCommandCommand)
+	cmd.Flag("diff-command", "Displays differences between two repository objects (files or directories)").Default(defaultDiffCommand()).Envar(svc.EnvName("KOPIA_DIFF")).StringVar(&c.diffCommandCommand)
 	cmd.Action(svc.repositoryReaderAction(c.run))
 
 	c.out.setup(svc)

cli/command_mount.go

@@ -24,6 +24,8 @@ type commandMount struct {
 	mountPreferWebDAV           bool
 	maxCachedEntries            int
 	maxCachedDirectories        int
+
+	svc appServices
 }
 
 func (c *commandMount) setup(svc appServices, parent commandParent) {
@@ -41,6 +43,7 @@ func (c *commandMount) setup(svc appServices, parent commandParent) {
 	cmd.Flag("max-cached-entries", "Limit the number of cached directory entries").Default("100000").IntVar(&c.maxCachedEntries)
 	cmd.Flag("max-cached-dirs", "Limit the number of cached directories").Default("100").IntVar(&c.maxCachedDirectories)
 
+	c.svc = svc
 	cmd.Action(svc.repositoryReaderAction(c.run))
 }
 
@@ -100,7 +103,7 @@ func (c *commandMount) run(ctx context.Context, rep repo.Repository) error {
 	// Wait until ctrl-c pressed or until the directory is unmounted.
 	ctrlCPressed := make(chan bool)
 
-	onCtrlC(func() {
+	c.svc.onCtrlC(func() {
 		close(ctrlCPressed)
 	})
 

cli/command_repository_change_password.go

@@ -16,7 +16,7 @@ type commandRepositoryChangePassword struct {
 
 func (c *commandRepositoryChangePassword) setup(svc advancedAppServices, parent commandParent) {
 	cmd := parent.Command("change-password", "Change repository password")
-	cmd.Flag("new-password", "New password").Envar("KOPIA_NEW_PASSWORD").StringVar(&c.newPassword)
+	cmd.Flag("new-password", "New password").Envar(svc.EnvName("KOPIA_NEW_PASSWORD")).StringVar(&c.newPassword)
 
 	c.svc = svc
 	cmd.Action(svc.directRepositoryWriteAction(c.run))

cli/command_repository_change_password_test.go

@@ -33,14 +33,12 @@ func (s *formatSpecificTestSuite) TestRepositoryChangePassword(t *testing.T) {
 	// at this point env2 stops working
 	env2.RunAndExpectFailure(t, "snapshot", "ls")
 
-	r3 := testenv.NewInProcRunner(t)
-
 	// new connections will fail when using old (default) password
-	env3 := testenv.NewCLITest(t, s.formatFlags, r3)
+	env3 := testenv.NewCLITest(t, s.formatFlags, testenv.NewInProcRunner(t))
 	env3.RunAndExpectFailure(t, "repo", "connect", "filesystem", "--path", env1.RepoDir, "--disable-repository-format-cache")
 
 	// new connections will succeed when using new password
-	r3.RepoPassword = "newPass"
+	env3.Environment["KOPIA_PASSWORD"] = "newPass"
 
 	env3.RunAndExpectSuccess(t, "repo", "connect", "filesystem", "--path", env1.RepoDir, "--disable-repository-format-cache")
 }

cli/command_repository_connect.go

@@ -22,7 +22,7 @@ type commandRepositoryConnect struct {
 func (c *commandRepositoryConnect) setup(svc advancedAppServices, parent commandParent) {
 	cmd := parent.Command("connect", "Connect to a repository.")
 
-	c.co.setup(cmd)
+	c.co.setup(svc, cmd)
 	c.server.setup(svc, cmd, &c.co)
 
 	for _, prov := range svc.storageProviders() {
@@ -61,16 +61,16 @@ type connectOptions struct {
 	disableFormatBlobCache  bool
 }
 
-func (c *connectOptions) setup(cmd *kingpin.CmdClause) {
+func (c *connectOptions) setup(svc appServices, cmd *kingpin.CmdClause) {
 	// Set up flags shared between 'create' and 'connect'. Note that because those flags are used by both command
 	// we must use *Var() methods, otherwise one of the commands would always get default flag values.
-	cmd.Flag("cache-directory", "Cache directory").PlaceHolder("PATH").Envar("KOPIA_CACHE_DIRECTORY").StringVar(&c.connectCacheDirectory)
+	cmd.Flag("cache-directory", "Cache directory").PlaceHolder("PATH").Envar(svc.EnvName("KOPIA_CACHE_DIRECTORY")).StringVar(&c.connectCacheDirectory)
 	cmd.Flag("content-cache-size-mb", "Size of local content cache").PlaceHolder("MB").Default("5000").Int64Var(&c.connectMaxCacheSizeMB)
 	cmd.Flag("metadata-cache-size-mb", "Size of local metadata cache").PlaceHolder("MB").Default("5000").Int64Var(&c.connectMaxMetadataCacheSizeMB)
 	cmd.Flag("max-list-cache-duration", "Duration of index cache").Default("30s").Hidden().DurationVar(&c.connectMaxListCacheDuration)
 	cmd.Flag("override-hostname", "Override hostname used by this repository connection").Hidden().StringVar(&c.connectHostname)
 	cmd.Flag("override-username", "Override username used by this repository connection").Hidden().StringVar(&c.connectUsername)
-	cmd.Flag("check-for-updates", "Periodically check for Kopia updates on GitHub").Default("true").Envar(checkForUpdatesEnvar).BoolVar(&c.connectCheckForUpdates)
+	cmd.Flag("check-for-updates", "Periodically check for Kopia updates on GitHub").Default("true").Envar(svc.EnvName(checkForUpdatesEnvar)).BoolVar(&c.connectCheckForUpdates)
 	cmd.Flag("readonly", "Make repository read-only to avoid accidental changes").BoolVar(&c.connectReadonly)
 	cmd.Flag("description", "Human-readable description of the repository").StringVar(&c.connectDescription)
 	cmd.Flag("enable-actions", "Allow snapshot actions").BoolVar(&c.connectEnableActions)

cli/command_repository_create.go

@@ -48,7 +48,7 @@ func (c *commandRepositoryCreate) setup(svc advancedAppServices, parent commandP
 	cmd.Flag("retention-mode", "Set the blob retention-mode for supported storage backends.").EnumVar(&c.retentionMode, blob.Governance.String(), blob.Compliance.String())
 	cmd.Flag("retention-period", "Set the blob retention-period for supported storage backends.").DurationVar(&c.retentionPeriod)
 
-	c.co.setup(cmd)
+	c.co.setup(svc, cmd)
 	c.svc = svc
 	c.out.setup(svc)
 

cli/command_restore.go

@@ -127,7 +127,7 @@ func (c *commandRestore) setup(svc appServices, parent commandParent) {
 	cmd.Flag("overwrite-files", "Specifies whether or not to overwrite already existing files").Default("true").BoolVar(&c.restoreOverwriteFiles)
 	cmd.Flag("overwrite-symlinks", "Specifies whether or not to overwrite already existing symlinks").Default("true").BoolVar(&c.restoreOverwriteSymlinks)
 	cmd.Flag("write-sparse-files", "When doing a restore, attempt to write files sparsely-allocating the minimum amount of disk space needed.").Default("false").BoolVar(&c.restoreWriteSparseFiles)
-	cmd.Flag("consistent-attributes", "When multiple snapshots match, fail if they have inconsistent attributes").Envar("KOPIA_RESTORE_CONSISTENT_ATTRIBUTES").BoolVar(&c.restoreConsistentAttributes)
+	cmd.Flag("consistent-attributes", "When multiple snapshots match, fail if they have inconsistent attributes").Envar(svc.EnvName("KOPIA_RESTORE_CONSISTENT_ATTRIBUTES")).BoolVar(&c.restoreConsistentAttributes)
 	cmd.Flag("mode", "Override restore mode").Default(restoreModeAuto).EnumVar(&c.restoreMode, restoreModeAuto, restoreModeLocal, restoreModeZip, restoreModeZipNoCompress, restoreModeTar, restoreModeTgz)
 	cmd.Flag("parallel", "Restore parallelism (1=disable)").Default("8").IntVar(&c.restoreParallel)
 	cmd.Flag("skip-owners", "Skip owners during restore").BoolVar(&c.restoreSkipOwners)

cli/command_server.go

@@ -28,10 +28,10 @@ type serverFlags struct {
 	serverPassword string
 }
 
-func (c *serverFlags) setup(cmd *kingpin.CmdClause) {
+func (c *serverFlags) setup(svc appServices, cmd *kingpin.CmdClause) {
 	cmd.Flag("address", "Server address").Default("http://127.0.0.1:51515").StringVar(&c.serverAddress)
-	cmd.Flag("server-username", "HTTP server username (basic auth)").Envar("KOPIA_SERVER_USERNAME").Default("kopia").StringVar(&c.serverUsername)
-	cmd.Flag("server-password", "HTTP server password (basic auth)").Envar("KOPIA_SERVER_PASSWORD").StringVar(&c.serverPassword)
+	cmd.Flag("server-username", "HTTP server username (basic auth)").Envar(svc.EnvName("KOPIA_SERVER_USERNAME")).Default("kopia").StringVar(&c.serverUsername)
+	cmd.Flag("server-password", "HTTP server password (basic auth)").Envar(svc.EnvName("KOPIA_SERVER_PASSWORD")).StringVar(&c.serverPassword)
 }
 
 type serverClientFlags struct {
@@ -41,18 +41,18 @@ type serverClientFlags struct {
 	serverCertFingerprint string
 }
 
-func (c *serverClientFlags) setup(cmd *kingpin.CmdClause) {
+func (c *serverClientFlags) setup(svc appServices, cmd *kingpin.CmdClause) {
 	c.serverUsername = "server-control"
 
-	cmd.Flag("address", "Address of the server to connect to").Envar("KOPIA_SERVER_ADDRESS").Default("http://127.0.0.1:51515").StringVar(&c.serverAddress)
-	cmd.Flag("server-control-username", "Server control username").Envar("KOPIA_SERVER_USERNAME").StringVar(&c.serverUsername)
-	cmd.Flag("server-control-password", "Server control password").PlaceHolder("PASSWORD").Envar("KOPIA_SERVER_PASSWORD").StringVar(&c.serverPassword)
+	cmd.Flag("address", "Address of the server to connect to").Envar(svc.EnvName("KOPIA_SERVER_ADDRESS")).Default("http://127.0.0.1:51515").StringVar(&c.serverAddress)
+	cmd.Flag("server-control-username", "Server control username").Envar(svc.EnvName("KOPIA_SERVER_USERNAME")).StringVar(&c.serverUsername)
+	cmd.Flag("server-control-password", "Server control password").PlaceHolder("PASSWORD").Envar(svc.EnvName("KOPIA_SERVER_PASSWORD")).StringVar(&c.serverPassword)
 
 	// aliases for backwards compat
 	cmd.Flag("server-username", "Server control username").Hidden().StringVar(&c.serverUsername)
 	cmd.Flag("server-password", "Server control password").Hidden().StringVar(&c.serverPassword)
 
-	cmd.Flag("server-cert-fingerprint", "Server certificate fingerprint").PlaceHolder("SHA256-FINGERPRINT").Envar("KOPIA_SERVER_CERT_FINGERPRINT").StringVar(&c.serverCertFingerprint)
+	cmd.Flag("server-cert-fingerprint", "Server certificate fingerprint").PlaceHolder("SHA256-FINGERPRINT").Envar(svc.EnvName("KOPIA_SERVER_CERT_FINGERPRINT")).StringVar(&c.serverCertFingerprint)
 }
 
 func (c *commandServer) setup(svc advancedAppServices, parent commandParent) {

cli/command_server_flush.go

@@ -13,7 +13,7 @@ type commandServerFlush struct {
 
 func (c *commandServerFlush) setup(svc appServices, parent commandParent) {
 	cmd := parent.Command("flush", "Flush the state of Kopia server to persistent storage, etc.")
-	c.sf.setup(cmd)
+	c.sf.setup(svc, cmd)
 	cmd.Action(svc.serverAction(&c.sf, c.run))
 }
 

cli/command_server_refresh.go

@@ -13,7 +13,7 @@ type commandServerRefresh struct {
 
 func (c *commandServerRefresh) setup(svc appServices, parent commandParent) {
 	cmd := parent.Command("refresh", "Refresh the cache in Kopia server to observe new sources, etc.")
-	c.sf.setup(cmd)
+	c.sf.setup(svc, cmd)
 	cmd.Action(svc.serverAction(&c.sf, c.run))
 }
 

cli/command_server_shutdown.go

@@ -15,7 +15,7 @@ type commandServerShutdown struct {
 
 func (c *commandServerShutdown) setup(svc appServices, parent commandParent) {
 	cmd := parent.Command("shutdown", "Gracefully shutdown the server")
-	c.sf.setup(cmd)
+	c.sf.setup(svc, cmd)
 	c.out.setup(svc)
 	cmd.Action(svc.serverAction(&c.sf, c.run))
 }

cli/command_server_source_manager_action.go

@@ -28,7 +28,7 @@ func (c *commandServerSourceManagerAction) setup(svc appServices, cmd *kingpin.C
 	cmd.Flag("all", "All paths managed by server").BoolVar(&c.all)
 	cmd.Arg("source", "Source path managed by server").StringVar(&c.source)
 
-	c.sf.setup(cmd)
+	c.sf.setup(svc, cmd)
 	c.out.setup(svc)
 }