* Enable linter: stylecheck
This enabled the stylecheck linter, but excludes some staticchecks for
now. The following are ignored because they will take a bit of time to
address, but the intention is to address them and remove them from the
exclusion list.
ST1000: at least one file in a package should have a package comment
ST1003: golang naming standards
ST1008: error should be returned as the last argument
ST1016: methods on the same type should have the same receiver name
ST1020: comment on exported function
* Address ST1015
For some reason this failed in CI but not locally. I can't figure out
why that check isn't happening locally. This just moves the switch
statements around so that the `default` is the first (or last) item.
* Adjust error string in test to match case-adjusted error
* Remove TestCloseTimeout
* 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
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)
The testhook post-test hooks only work if you use a TestMain to
invoke them, otherwise the cleanups can be registered but never
actually get run. This deletes the etcd sockets, and temp
directories, that we created from our test runs. We also fix
the test creating a temp file directly to create it in a TempDir
(which gets cleaned up after the test), and fix the name of the
top-level tests displayed in TestMain.
When doing tests, we create a ton of one-off clusters. This
turns out to be expensive and slow. Fixing it is surprisingly hard.
Fundamentally: If we're sharing clusters, we need to use different
indexes for each test, to avoid clashes. This changes index names.
As a side-effect, this reorders many partition-based things, like
the order keys are returned in. Thus, to fix this, we change a lot
of tests to no longer depend on the *order* in which strings are
returned.
Having done that, we can also discard the ModHasher behavior, since
that only existed to allow us to reliably predict partitioning.
The basic design is as follows: Instead of a cluster being a
[]*Command, a "shareable" cluster is now a []*Command plus some
flags, and a "cluster" is a pointer to a possibly-shared cluster,
plus a link to the specific test using this specific cluster,
and correspondingly, its test name suitably coerced to be a valid
index name prefix.
The "test.Cluster" object now has methods to allow retrieving an
index name, and also implemnts fmt.Formatter to let you use,
e.g., `%i` with it in Sprintf to get "the index name, plus an i".
(This works for everything but %p and %T.)
This allows us to consistently rework all the many things that
use index names in a persistent way.
We also have `MustUnshared` and `MustRunUnsharedCluster` methods
which allow us to specify that a given test needs its own cluster
for some reason. For instance, the tests that want to run backups
need their own isolated cluster, and the tests that want to close
or reopen nodes need their own cluster because a reopened cluster
won't have working GRPC for some reason.
On "closing" a shared cluster (actually the test-specific wrapper
that reflects a given sharing), we delete any indexes starting with
that test's index name prefix. Otherwise, the huge pile of open
indexes prevents `go test -race` from working on MacOS, where we
run out of address space too quickly.
This is fairly enormous but most of the individual changes are
fairly trivial things like replacing the string "i" with "c.Idx()".
We also tweaked a test that failed for me a couple of times to
not depend on sort order.
