Skip to content

Commit

Permalink
serverutils: move an assertion to a better place
Browse files Browse the repository at this point in the history
Prior to this patch, the assertion on the value of DefaultTestTenant
was inside `ShouldStartDefaultTestTenant()`. However, this function
is only ever called from `StartServer`; it is not called from
`NewServer()`. This means that tests that call `NewServer` directly do
not get the benefit of the validity check.

This patch moves the assertion to the better place.

Release note: None
  • Loading branch information
knz committed Sep 1, 2023
1 parent 79a76e2 commit 3fdc2ab
Showing 1 changed file with 4 additions and 6 deletions.
10 changes: 4 additions & 6 deletions pkg/testutils/serverutils/test_server_shim.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,6 @@ var PreventStartTenantError = errors.New("attempting to manually start a virtual
func ShouldStartDefaultTestTenant(
t TestLogger, baseArg base.DefaultTestTenantOptions,
) (retval base.DefaultTestTenantOptions) {
defer func() {
if !(retval.TestTenantAlwaysEnabled() || retval.TestTenantAlwaysDisabled()) {
panic(errors.AssertionFailedf("programming error: no decision was actually taken"))
}
}()

// Explicit cases for enabling or disabling the default test tenant.
if baseArg.TestTenantAlwaysEnabled() {
return baseArg
Expand Down Expand Up @@ -243,6 +237,10 @@ func NewServer(params base.TestServerArgs) (TestServerInterface, error) {
return nil, errors.AssertionFailedf("TestServerFactory not initialized. One needs to be injected " +
"from the package's TestMain()")
}
tcfg := params.DefaultTestTenant
if !(tcfg.TestTenantAlwaysEnabled() || tcfg.TestTenantAlwaysDisabled()) {
return nil, errors.AssertionFailedf("programming error: DefaultTestTenant does not contain a decision\n(maybe call ShouldStartDefaultTestTennat?)")
}

srv, err := srvFactoryImpl.New(params)
if err != nil {
Expand Down

0 comments on commit 3fdc2ab

Please sign in to comment.