Fix a unit test regression when using non-default SSH storage backends.

Review Request #11006 — Created May 4, 2020 and submitted

Information

Review Board
release-3.0.x

Reviewers

SSH-related unit tests often involve shelling out to rbssh, which, as of
a recent change in the un-released 3.0.18, now looks up the proper
backend to use in the database. However, since rbssh is its own process,
it doesn't have access to the in-memory test database that the rest of
Review Board is running in, and will pick up the storage backend
configured in the developer's database. This can cause unit tests to
fail, since the environment is set up incorrectly for those tests.

This change introduces a workaround, which is a new
$RBSSH_STORAGE_BACKEND environment variable for rbssh that will take
precedence over any other configured backends. This value, if provided,
will be passed in to SSHClient using the new storage_backend
parameter, and that will short-circuit the storage loading.

Review Board does not set this during normal usage. It only sets it
during unit test runs. However, it might be worth exploring setting this
in a future change, in order to reduce rbssh startup time, but that will
be a separate project.

This also provides a better, more consistent environment variable name
for debugging rbssh, called $RBSSH_DEBUG. Both this and the old
$DEBUG_RBSSH are supported.

Unit tests pass with and without Power Pack's SSHDB enabled locally.

Verified that rbssh from within Review Board and on the command line
was respecting the configured SSH storage backend.

Summary ID
Fix a unit test regression when using non-default SSH storage backends.
SSH-related unit tests often involve shelling out to rbssh, which, as of a recent change in the un-released 3.0.18, now looks up the proper backend to use in the database. However, since rbssh is its own process, it doesn't have access to the in-memory test database that the rest of Review Board is running in, and will pick up the storage backend configured in the developer's database. This can cause unit tests to fail, since the environment is set up incorrectly for those tests. This change introduces a workaround, which is a new `$RBSSH_STORAGE_BACKEND` environment variable for rbssh that will take precedence over any other configured backends. This value, if provided, will be passed in to `SSHClient` using the new `storage_backend` parameter, and that will short-circuit the storage loading. Review Board does not set this during normal usage. It only sets it during unit test runs. However, it might be worth exploring setting this in a future change, in order to reduce rbssh startup time, but that will be a separate project. This also provides a better, more consistent environment variable name for debugging rbssh, called `$RBSSH_DEBUG`. Both this and the old `$DEBUG_RBSSH` are supported.
9e1b91f8c26d589eee67412283774f3b748ef41c
david
  1. Ship It!
  2. 
      
chipx86
Review request changed
Status:
Completed
Change Summary:
Pushed to release-3.0.x (56a7182)