The robustness CI jobs are failing with "flag provided but not defined:
-bin-dir" because the test path includes subpackages (./tests/robustness/...)
that don't register this flag.
PR 20962 changed the test invocation to use ./tests/robustness/... which
runs tests in all subpackages. The validate/ and model/ subpackages contain
unit tests that don't import framework/e2e (which registers -bin-dir), so
they fail when this flag is passed.
PR 21039 attempted to fix the issue but only changed the test runner
function, not the package path.
This change removes the /... suffix to only run tests in the main
robustness package, restoring the original behavior.
Signed-off-by: Davanum Srinivas <davanum@gmail.com>
Remove running the tests for all the packages under ./tests/robustness.
Restore the old behavior of running only the tests from the top-level
robustness package.
Signed-off-by: Ivan Valdes <ivan@vald.es>
Introduce the new functions: run_go_tests and
run_go_test_expanding_packages, which implement (without Shellcheck
exceptions) the old behavior from go_test, simplifying the arguments
received.
Replaced the three possible values mode with:
1. keep_going: By using the KEEP_GOING_TESTS environment variable. This
mode expanded the relative modules; it can be run by using
run_go_tests_expanding_packages.
2. parallel: It worked without expanding the relative modules with the
absolute modules. Therefore, this mode can be run directly using
run_go_tests.
3. fail_fast: By adding an argument "-failfast", and not setting the
KEEP_GOING_TESTS environment variable.
The argument flags_for_package_func can be replaced by regular arguments
passed to the function.
These two changes simplify the complexity of the go test wrapper
function and allow further customization in the callers.
The JUnit XML generation works the same way, by reading the
JUNIT_REPORT_DIR or ARTIFACTS environment variable. It generates the
same reports.
Temporarily introduce the get_junit_filename_prefix function, as a
replacement for junitFilenamePrefix. The latter will be removed once all
test targets are using the new functions.
Finally, update the unit tests function to make use of the new
functions, and allow running the tests by using the workspace (i.e.,
there's no need to change directories anymore).
Signed-off-by: Ivan Valdes <ivan@vald.es>
Remove the custom environment variable GOLANG_TEST_SHORT.
It is currently used by Main tests (TestMain), as we can't skip tests
because testing.M doesn't implement testing.TB. It is possible to do a
clean os.Exit(0) (current behavior), but calling testing.Short() fails
because this function expects flags to be parsed before.
So, it is possible to remove the custom behavior (GOLANG_TEST_SHORT) by
parsing flags (if required) before calling testing.Short(), then
immediately exit if the result is true (-short flag is set).
Signed-off-by: Ivan Valdes <ivan@vald.es>
Follow the approach of a script per Makefile target. Rename it to
shell_ws for consistency with the verify target.
Signed-off-by: Ivan Valdes <ivan@vald.es>
We're already calling golangci-lint --fix. We require the gofmt
formatter, and it supports autofix. Therefore, running go fmt manually
after golangci-lint --fix runs doesn't have any effect.
Signed-off-by: Ivan Valdes <ivan@vald.es>
Use the new workspace helper functions.
By moving it to a test.sh pass, we can chain it in the fix Makefile
target prerequisites, and it can be called before update-go-workspace.
Signed-off-by: Ivan Valdes <ivan@vald.es>
Use a new function for commands that need to change directories to the
Go workspace module sub directory. For example, go mod doesn't receive
arguments, it needs to run from the directory that has the go.mod file.
Signed-off-by: Ivan Valdes <ivan@vald.es>
Currently, we need to force word splitting, as we need to pass multiple
arguments, not a single one, to the command that will run using the
workspace modules.
To properly remove this exception, we need to use an array. The problem
with Bash arrays is that you can't return them in a function. So, the
way to build an array is by reference. Otherwise, we'll end up
rebuilding the arrays every time we need to call these functions.
Signed-off-by: Ivan Valdes <ivan@vald.es>
This is now replaced by the golangci-lint goheader linter.
Deletes the bash helper go_srcs_in_module, as it is not used anymore by
any other static check functions.
Signed-off-by: Ivan Valdes <ivan@vald.es>
The gofmt static check is already part of golangci-lint's gofmt
formatter. Therefore, it's a duplicate check. We can simplify and let
golangci-lint do the heavy lifting.
Signed-off-by: Ivan Valdes <ivan@vald.es>
The tool github.com/appscodelabs/license-bill-of-materials fails during
the first run in a recently cloned/fresh repository. Because it captures
the output, and the first run shows the download of dependencies. In
8b02416301, we intentionally added a new
execution before evaluating whether the BOM is up to date. However, this
is polluting the logs as it has the expected failure. Redirecting the
output to /dev/null alleviates the issue.
Signed-off-by: Ivan Valdes <ivan@vald.es>
Using shadow from golangci-lint doesn't lint generated files
(protobufs), so it's safe to include all the files.
Signed-off-by: Ivan Valdes <ivan@vald.es>
Adds workspace_relative_modules, and run_for_all_workspace_modules which
runs a given function for all the workspace module from the top-level of
the repository, without iterating by module/directory.
Signed-off-by: Ivan Valdes <ivan@vald.es>
Introduce a new `verify-go-workspace` make target executed by the
`verify` target. The pull-etcd-verify presubmit prow job will execute
it. It ensures that the Go workspace (`go.work` and `go.work.sum`) is in
sync.
Signed-off-by: Ivan Valdes <ivan@vald.es>
Introduce a new Go workspace that references all the current submodules.
To preserve the current behavior, point all of the current
FORBIDDEN_DEPENCENCY virtual dependencies to the same path.
Add scripts/update_go_workspace.sh that generates the Go workspace file
(go.work) based on the submodules found in the project (excluding the
tools) and creates the go.work.sum file after running go mod download.
Added this script to the fix Makefile target. Even though, the number of
modules in the etcd repository is small, by adding an automated way of
updating the go.work modules future proofs the project in case there are
new modules or removal of them.
Remove the workaround to forbid dependencies through the
FORBIDDEN_DEPENDENCY replace. This can be done through a linter rather
than at the go.mod level. Using the go.mod approach also brings issues
with the workspace, as all the dependencies (including replaces) need to
have a consistent version and point to the same place.
Co-authored-by: Tim Hockin <thockin@google.com>
Signed-off-by: Ivan Valdes <ivan@vald.es>
Ensure that GOTOOLCHAIN is set by the time it runs, to ensure the binary
gets compiled with the right Go version.
Signed-off-by: Ivan Valdes <ivan@vald.es>
