tests: Document robustness tests #14838

serathius · 2022-11-23T15:28:24Z

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

cc @ahrtr @ptabor @spzala
Do you have any suggestion on topics to cover in the README?

tests/linearizability/README.md

serathius · 2023-02-09T19:35:07Z

cc @lavacat @geetasg

geetasg · 2023-02-09T22:50:22Z

It would be nice to have a contributor section that guides on how to extend the model.

tests/linearizability/README.md

serathius · 2023-03-18T12:09:00Z

Finally got to writing down the docs.
cc @ahrtr @ptabor @logicalhan @chaochn47 @jmhbnz

logicalhan

/lgtm
/approve

tests/robustness/README.md

ahrtr · 2023-03-21T06:02:56Z

Makefile

+.PHONY: reproduce-issue14370
+reproduce-issue14370: bin/etcd-v3.5.4-failpoints
+	cp bin/etcd-v3.5.4-failpoints ./bin/etcd
+	GO_TEST_FLAGS='-v --run=TestRobustness/Issue14370 --count 100 --failfast' make test-robustness
+
+.PHONY: reproduce-issue13766
+reproduce-issue13766: bin/etcd-v3.5.2-failpoints
+	cp bin/etcd-v3.5.2-failpoints ./bin/etcd
+	GO_TEST_FLAGS='-v --run=TestRobustness/Issue13766 --count 100 --failfast' make test-robustness
+
+.PHONY: reproduce-issue14685
+reproduce-issue14685: bin/etcd-v3.5.5-failpoints
+	cp bin/etcd-v3.5.5-failpoints ./bin/etcd
+	GO_TEST_FLAGS='-v --run=TestRobustness/Issue14685 --count 100 --failfast' make test-robustness


I recall I raised a comment somewhere that the top level Makefile isn't a good place to just demonstrate the ability of the robustness test, and @ptabor suggested to move them into a script or doc inside a sub-directory.

I still have the same comment. Let's keep the top level Makefile as simple & clean as possible.

Don't agree with this. Robustness test targets touch files whole repo making it impossible/bothersome to put it in subdirectory. For example moving proposed targets to tests/robustness/Makefile would result in all paths needing to be prefixed cd ../.. &&.

Let's keep the top level Makefile as simple & clean as possible.

Don't agree, each Makefile target should be simple and clean, however it doesn't mean that root Makefile should have limited list of targets. I would argue that makefile scales pretty well with number of targets, assuming that we maintain a proper structure. For example https://github.com/kubernetes-sigs/metrics-server/blob/master/Makefile

For me the problem is that we want to avoid adding more targets to root Makefile for it to remain simple & clean, however I expect that it will cause reverse effect. Make code more complicated by introducing hard to follow dependencies. Proposed targets cannot be simply moved to subdir as they depend on targets (like GOFAIL_VERSION, gofail-enable, gofail-disable, test-robustness) and directories (like ./bin) from root directory.

Simple and clean solution here is just put those targets in root directory. Distributing makefile targets between subdirectories is the more complicated option. Makefile was just not designed for hierarchy.

I didn't propose to add another Makefile in a sub-directory.

Oh, I misread.

@ptabor suggested to move them into a script or doc inside a sub-directory.

I still would prefer to split the files. Even for sake of avoiding long content in the main directory.
My approach would be following:

Decide how we want to run robustness tests against historical versions long-term:

Address in a similar way the historical cases the requires very specific etcd revision for repro.

I think that the build code should be as much as possible versioned together with the System-Under-Test. In this case for 3.4.x, 3.5.x, 3.6.0+ I would cherry pick to the main Makefile a rule that allows to build:

make build-with-failpoints FAILPOINTS=${GO_FAIL_VERSION}

that as an outcome would produce ./bin/etcd-failpoints (and potentially the other binaries)

This would allow us to have in the main file a generic rule:

./tmp/etcd-${}/bin/etcd-failpoints: (clones the repo and calls the nested build) robustness-test-{...}: ./tmp/etcd-{...}/bin/etcd-failpoints ETCD_BIN=./tmp/etcd-{...}/bin/etcd-failpoints make test-robustness

For the specific repro, I would consider using the Makefile's include (https://www.gnu.org/software/make/manual/html_node/Include.html)

So having the code in ./tests/robustness/makefile.mk and including it from the main MakeFile.

I think that such inclusion does not causes the cd ../.. problems still allowing to encapsulate the robustness related code with the robustness tests.

Moved robustness test targets to ./tests/robustness/makefile.mk. However I would wait with unifying build with failpoints targets for older branches. I think it's an overkill for now and we can always improve it later.

Signed-off-by: Marek Siarkowicz <siarkowicz@google.com>

serathius · 2023-03-28T13:56:42Z

PTAL @ahrtr @ptabor

ptabor · 2023-03-28T14:06:19Z

tests/robustness/README.md

+    * `GO_TEST_FLAGS` - to pass additional arguments to `go test`. 
+      It is recommended to run tests multiple times with failfast enabled. this can be done by setting `GO_TEST_FLAGS='--count=100 --failfast'`.
+    * `EXPECT_DEBUG=true` - to get logs from the cluster.
+    * `RESULTS_DIR` - to change location where results report will be saved.


What's the default for RESULTS_DIR ? within the test's tmp directory that is cleaned after success ?

No, robustness tests specifically create its own directory in tmp that is not created via t.TmpDir(). It even copies db files there to ensure they are preserved.

serathius · 2023-03-28T14:19:24Z

scripts/test.sh

@@ -126,7 +126,7 @@ function e2e_pass {

 function robustness_pass {
  # e2e tests are running pre-build binary. Settings like --race,-cover,-cpu does not have any impact.
-  run_for_module "tests" go_test "./robustness/..." "keep_going" : -timeout="${TIMEOUT:-30m}" "${RUN_ARG[@]}" "$@"
+  run_for_module "tests" go_test "./robustness" "keep_going" : -timeout="${TIMEOUT:-30m}" "${RUN_ARG[@]}" "$@"


Subdirs include normal unit tests that are not aware of --bin-dir flag. Unit tests are already run via make test-unit

serathius · 2023-03-28T14:19:56Z

scripts/test_lib.sh

@@ -315,7 +315,7 @@ function go_test {
    additional_flags=$(${flags_for_package_func} ${pkg})

    # shellcheck disable=SC2206
-    local cmd=( go test ${goTestFlags} ${additional_flags} "$@" ${pkg} )
+    local cmd=( go test ${goTestFlags} ${additional_flags} ${pkg} "$@" )


Flags defined by tests like --bin-dir need to be passed after package.

serathius marked this pull request as draft November 23, 2022 15:28

serathius force-pushed the linearizability-docs branch 2 times, most recently from a115242 to 1478354 Compare November 24, 2022 16:12

ptabor reviewed Nov 25, 2022

View reviewed changes

tests/linearizability/README.md Outdated Show resolved Hide resolved

tests/linearizability/README.md Outdated Show resolved Hide resolved

tests/linearizability/README.md Outdated Show resolved Hide resolved

geetasg reviewed Feb 9, 2023

View reviewed changes

tests/linearizability/README.md Outdated Show resolved Hide resolved

tests/linearizability/README.md Outdated Show resolved Hide resolved

tests/linearizability/README.md Outdated Show resolved Hide resolved

serathius changed the title ~~tests: Document linearizability tests~~ tests: Document robustness tests Mar 18, 2023

serathius force-pushed the linearizability-docs branch from 1478354 to 044051c Compare March 18, 2023 12:07

serathius marked this pull request as ready for review March 18, 2023 12:08

logicalhan approved these changes Mar 20, 2023

View reviewed changes

ahrtr reviewed Mar 21, 2023

View reviewed changes

serathius force-pushed the linearizability-docs branch from 044051c to 106070f Compare March 28, 2023 13:04

tests: Document robustness tests

d03ac88

Signed-off-by: Marek Siarkowicz <siarkowicz@google.com>

serathius force-pushed the linearizability-docs branch from 106070f to d03ac88 Compare March 28, 2023 13:08

ptabor approved these changes Mar 28, 2023

View reviewed changes

serathius commented Mar 28, 2023

View reviewed changes

serathius merged commit 5223d09 into etcd-io:main Mar 28, 2023

serathius deleted the linearizability-docs branch June 15, 2023 20:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests: Document robustness tests #14838

tests: Document robustness tests #14838

serathius commented Nov 23, 2022

serathius commented Feb 9, 2023

geetasg commented Feb 9, 2023

serathius commented Mar 18, 2023

logicalhan left a comment

ahrtr Mar 21, 2023 •

edited

Loading

serathius Mar 27, 2023

serathius Mar 27, 2023

ahrtr Mar 27, 2023

serathius Mar 28, 2023

ptabor Mar 28, 2023

serathius Mar 28, 2023

serathius commented Mar 28, 2023

ptabor Mar 28, 2023

serathius Mar 28, 2023 •

edited

Loading

serathius Mar 28, 2023

serathius Mar 28, 2023

tests: Document robustness tests #14838

tests: Document robustness tests #14838

Conversation

serathius commented Nov 23, 2022

serathius commented Feb 9, 2023

geetasg commented Feb 9, 2023

serathius commented Mar 18, 2023

logicalhan left a comment

Choose a reason for hiding this comment

ahrtr Mar 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

serathius commented Mar 28, 2023

Choose a reason for hiding this comment

serathius Mar 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahrtr Mar 21, 2023 •

edited

Loading

serathius Mar 28, 2023 •

edited

Loading