roachtest: fix parameters passed to require.NoError

[renatolabs]

When context is passed to an assertion, the parameters must be a string format, followed by arguments (as you would in a call to fmt.Sprintf). The previous code would panic trying to cast int to string.

Informs #95416

Release note: None

[cockroach-teamcity]

This change is 

[yuzefovich]

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @herkolategan and @srosenberg)

[srosenberg]

I wonder if this could have been picked up by one of the linters?

Reviewable status: complete! 2 of 0 LGTMs obtained (waiting on @herkolategan and @renatolabs)

---

pkg/cmd/roachtest/tests/multitenant_distsql.go line 142 at r1 (raw file):

					// procede to report error
				}
				require.NoError(t, err, "instance idx = %d, iter = %d", li, iter)

It essentially fails the typecast,

if len(msgAndArgs) > 1 {
	return fmt.Sprintf(msgAndArgs[0].(string), msgAndArgs[1:]...)
}

[herkolategan]

Reviewable status: complete! 3 of 0 LGTMs obtained (waiting on @renatolabs)

[renatolabs]

I wonder if this could have been picked up by one of the linters?

We don't have any linters checking for that (not sure if there's something out there that we could use, a quick Google search didn't reveal anything).

bors r=yuzefovich,srosenberg,herkolategan

[srosenberg]

We don't have any linters checking for that (not sure if there's something out there that we could use, a quick Google search didn't reveal anything).

fmtsafe.Analyzer [1] does essentially what we want in this case, no?

[1]

cockroach/pkg/testutils/lint/passes/fmtsafe/fmtsafe.go

Line 32 in 6fc1022

- that the format string in Infof(), Errorf() and similar calls is a

[renatolabs]

fmtsafe.Analyzer [1] does essentially what we want in this case, no?

Not quite? It is similar, but it makes two assumptions that wouldn't work well for this purpose:

• it ensures the format argument passed to the fmt-like fuctions is a constant string. This seems overly restrictive for this use case.
• It's equipped to verify functions with the following signature f(format string, args ...interface{}). The require-style calls here have the format f(msgAndArgs ...interface{}), where the "format" is part of the variadic args.

In other words, we'd have to teach the semantics of require to the linter, which is not ideal: we'd have to change the description of the linter to:

checks that log and error functions don't leak PII -- and also checks `require` calls while at it

That said, this is a good experiment for the tools you mentioned recently (revive / ruleguard); I'll play with this later.

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)