Skip to content

Take the SSH defaults off direct environment reads #515

Description

@josegonzalez

Description of feature

t.Setenv panics in a parallel test, so every test that has to arrange an environment variable is permanently serial. After #502 those are the only serial tests left: eight in subprocess and the five in commands/ssh_flags_test.go. Three separate couplings are behind them, and they do not all deserve the same answer.

Four of the eight are already dead. TestBuildSshArgvQuotesRemoteArgs and three siblings clear DOKKU_SUDO and DOKKU_SSH_ACCEPT_NEW_HOST_KEYS before calling buildSshArgv. #504 moved both onto the explicit Target that function now takes, so subprocess/ssh.go has not read either variable since. The clearing protects nothing and can simply go.

defaultSshUser (subprocess/ssh.go:157) reads USER and LOGNAME to decide who to ssh as when a host carries no user@. Two tests set them. The read is a pure function of the environment, so a lookup parameter defaulting to os.Getenv is enough to let a test state its own answers.

resolveSshFlags (commands/ssh_flags.go:25) reads DOKKU_HOST, DOKKU_SUDO and DOKKU_SSH_ACCEPT_NEW_HOST_KEYS as the fallbacks behind --host, --sudo and --accept-new-host-keys. Same shape, same fix, and it is the whole reason commands/ssh_flags_test.go is the last serial file in that package.

Two tests should stay serial, and it is worth writing down why so nobody tries again:

  • TestCallExecCommandInheritsProcessEnv asserts that a child process inherits docket's environment. Its subject is the process environment; a stubbed lookup would assert nothing.
  • TestCallExecCommandTraceLogIsMasked sets DOKKU_TRACE=1, but it also swaps the standard logger's output with log.SetOutput to read the trace back. Decoupling the environment alone would not free it, and the log sink is a second, larger question.

Nothing here is a bug. os.Getenv at these three sites is correct for the CLI; the cost is only that the tests exercising them cannot run beside anything else.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions