* Add (commented out) linters that we should introduce
I went through the available linters and added (commented out) the ones
I think we should work on in the near term. In other words, fix them,
then uncomment them so they are enabled in CI.
* linter: errchkjson
* linter: ineffassign
* linter: gosimple
* linter: errname
The testing package is full of subtle magic, and one of the most
subtle is this: t.Logf, etcetera, all write to a buffer which is
then displayed after the test is run. Which means that, if you
exit, the buffer is never displayed. This means that, if a test
case can fail in a way that causes an instant exit, you don't
hit defers, you don't get your log messages, you just get a mysterious
exit of the process.
We have two cases where backup commands were calling log.Fatal
instead of returning an error. The error in question is displayed
correctly and informatively if returned, so we return it.
We also have one case where we were using os.Exit to avoid a
deadlock. Instead, we make the thing that would deadlock
conditional on the test not having failed. In the event that
the test fails, we now print our failure message correctly,
then also report an unclosed cluster. That's fine.
* Prevent file corruption when writing tar backup to stdout
FB-1794
Tar backups written to stdout were coming out corrupt. This turned
out to be due to log messages getting written to stdout and ending
up in the tar file. We now check to see if the tar file and the log
are both going to stdout, and if they are, send the logs to stderr
instead.
Testing did not have any kind of consistency or validity check. We
now compare a tar file sent to a file and a tar file sent to stdout
to make sure they're the same. This does not guarantee correctness
but does at least catch this form of corruption.
* trying different index name
Co-authored-by: tgruben <tgruben@gmail.com>
Co-authored-by: Todd Gruben <todd@molecula.com>
(cherry picked from commit a8996a149d)
stdout/stderr around
A lot of functions in the cmd and ctl packages were passing these
around and barely using them. Replaced them with a logger for most
functions. Some functions get an io.Writer instead so that their
tests can find the output they're looking for.
More cleanup on fb-1766: reworked the tests that were using io.Pipe
or os.Pipe to check their results so they now use a bytes.Buffer.
Unexported some variables that didn't need to be exported.
Fixed NewConfigCommand to use the provided stderr, not os.Stderr.
Added tests for rbf_dump, rbf_page, and keygen, since those weren't
being tested at all.
Added chksum_test, final cleanup.
(cherry picked from commit f627199acb)
the BackupTar and RestoreTar functionality was ignoring provided
readers, which doesn't matter for real usage but breaks tests
by making them dump raw tar binaries to stdout.
(cherry picked from commit 93153a97db)
Cobra automatically displays usage messages, and also a gratuitous
"Error: [...]" line in some cases, when any error at all occurs
running a command. To suppress the usage message, you have to set
cmd.SilenceUsage to true. But the code that would do this doesn't
have access to it. To address this, we introduce a category of
"usage error", implemented with stdlib error wrapping (%w) and
use errors.Is to check for it. There's also utility functions
to do this checking automatically, or indeed, to handle wrapping
of the ctl.SomethingCommand and handle running it with a suitable
context and everything.
In fact, several of the places we're checking for usage errors,
we can never actually report one, but we're checking consistently
so that if we want to report usage errors, we can.
For instance, server.Start and (dax)server.Start don't ever
return usage errors, right now, but we're checking their responses
anyway.
(cherry picked from commit c681642734)
This fixes the OriginalIP and RequestUserID in the main featurebase
package, and the Access and Refresh tokens, the UserInfo, and the
[]string of Indexes passed with context.Context(s) in the authn package.
An empty struct was used for all of these keys (and relevant helper
functions we added) to avoid allocations where possible while still
using the context functionality.
Some of the logic in the server.GetIndexes function was fixed.
(cherry picked from commit 0f5a56c958)
