Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: store test cluster logs and improve test readme [DET-3269] #657

Merged
merged 6 commits into from
Jun 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions webui/tests/.gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
node_modules/
results/
*.log
32 changes: 29 additions & 3 deletions webui/tests/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ two SPAs, Elm and React, and in some sense the whole det cluster.
We provide a test script to take care of depdencies around Cypress commands.
Let's call this the "test script" for the rest of the this document.

## Requirements
## Running the Tests

When the tester loads up it loads the WebUI through a given master address
(`$HOST`) and port (`$PORT`)
Expand All @@ -28,7 +28,7 @@ Once the cluster is ready and accessible run the tests:
Note that it's is important that one immediately follows the other since the
pre-e2e-tests target starts some experiments.

### Using the bundled cluster manager
### Using the Bundled Cluster Manager

For ease of use and quicker local testing we provide an option to set up and
tear down such a cluster through the test script.
Expand All @@ -37,8 +37,34 @@ Before the tests can be started we need to build the cluster including the WebUI
to make sure the served WebUI and cluster in general are up to date.

Issue the following command:
`./bin/e2e-tests.py e2e-tests` which in turn will:
`./bin/e2e-tests.py e2e-tests` (or `make test`) which in turn will:

1. Set up a test cluster
2. Run the Cyrpess tests `Cypress run`
3. Tear down the cluster and clean up in case of errors

## Test Development

Use `make dev-tests` to set up for test development and then proceed to add new
tests suites or update and rebuild the WebUI artifacts to see changes in tests.

### Debugging Test Issues

For reproducing and catching test flakes there is a simple helper script `./bin/try-for-flake.sh`.
Just executing the script will run `make test` over and over until it hits a error.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: add that you can pass in a parameter to limit the number of times to run tests. eg) ./bin/try-for-flake.sh 30 to run make test 30 times.

Copy link
Member Author

@hamidzr hamidzr Jun 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that would be helpful but the script doesn't support this option we can add that (separately) though.


To speed up this process you can:

1. try to avoid the whole cluster set up and tear down on each iteration. If the suspected tests are
re-runable or you have a way of setting up and tearing down the cluster that is faster than the provided
`test-cluster` refer to the "Re-runnable Tests" section.
2. limit the test scope:
- use `.skip` on each unwanted test suite or `.only` on the target suite.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: does this actually work with our cypress setup? It looks like it's a mix bag. I tried applying .only to the very first test in 02-loginout.spec.ts and it ran all the other test files fully, and when it came to 02-loginout.spec.ts it did run just the first test where I applied .only... Is that the correct behavior or is there a weird setup going on?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update: here's what worked for blocking full test files. If you apply x to the top level describe (xdescribe) it stops the entire describe block in that file from running. So to isolate the tests I want to run, I need to add x to all the other files.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was mixed bag last time I used it. from my googling, it's about the Jest version that Cypress is using. It is supposed to work so I think it's safe to have it in the readme. I usually just delete the unwanted test suites.
cypress-io/cypress#5441
cypress-io/cypress#632
cypress-io/cypress#5345

- temporarily delete the unwanted test suites

#### Re-runnable Tests

By default the tests are not re-runnable without meeting the conditions in the "Running the Tests" section.
However if you can get the tests to a state where they are re-runnable without the need for the cluster to be
reset, pass in `true` to the test flake script to instruct it to skip the cluster set up and tear down and
greatly speed up the process
32 changes: 23 additions & 9 deletions webui/tests/bin/e2e-tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import sys
from typing import List

RESULTS_DIR_NAME = "results"
logger = logging.getLogger("e2e-tests")

root = subprocess.check_output(
Expand All @@ -18,8 +17,10 @@
root_path = pathlib.Path(root)
webui_dir = root_path.joinpath("webui")
tests_dir = webui_dir.joinpath("tests")
results_dir = tests_dir.joinpath("results")
test_cluster_dir = tests_dir.joinpath("test-cluster")

CLUSTER_CMD_PREFIX = ["make", "-C", "test-cluster"]
CLUSTER_CMD_PREFIX = ["make", "-C", str(test_cluster_dir)]

CLEAR = "\033[39m"
BLUE = "\033[94m"
Expand All @@ -31,8 +32,8 @@ def run(cmd: List[str], config) -> None:
return subprocess.check_call(cmd, env=config["env"])
hamidzr marked this conversation as resolved.
Show resolved Hide resolved


def run_forget(cmd: List[str], config) -> None:
return subprocess.Popen(cmd, stdout=subprocess.DEVNULL)
def run_forget(cmd: List[str], logfile, config) -> None:
return subprocess.Popen(cmd, stdout=logfile)


def run_ignore_failure(cmd: List[str], config):
Expand All @@ -42,10 +43,15 @@ def run_ignore_failure(cmd: List[str], config):
pass


def setup_cluster(config):
def setup_results_dir(config):
run_ignore_failure(["rm", "-r", str(results_dir)], config)
run(["mkdir", "-p", str(results_dir)], config)


def setup_cluster(logfile, config):
logger.info("setting up the cluster..")
run(CLUSTER_CMD_PREFIX + ["start-db"], config)
cluster_process = run_forget(CLUSTER_CMD_PREFIX + ["run"], config)
cluster_process = run_forget(CLUSTER_CMD_PREFIX + ["run"], logfile, config)
time.sleep(5) # FIXME add a ready check for master
logger.info(f"cluster pid: {cluster_process.pid}")
return cluster_process
Expand All @@ -63,14 +69,17 @@ def teardown_cluster(config):
@contextmanager
def det_cluster(config):
try:
yield setup_cluster(config)
log_path = str(test_cluster_dir.joinpath("cluster.stdout.log"))
with open(log_path, "w") as f:
yield setup_cluster(f, config)

finally:
teardown_cluster(config)


def pre_e2e_tests(config):
run_ignore_failure(["rm", "-r", str(tests_dir.joinpath(RESULTS_DIR_NAME))], config)
# TODO add a check for cluster condition
setup_results_dir(config)
run(
["python", str(tests_dir.joinpath("bin", "createUserAndExperiments.py"))],
config,
Expand Down Expand Up @@ -120,6 +129,7 @@ def cypress_open(config):


def e2e_tests(config):
setup_results_dir(config)
with det_cluster(config):
pre_e2e_tests(config)
run_e2e_tests(config)
Expand Down Expand Up @@ -168,7 +178,11 @@ def main():
help_msg = f"operation must be in {sorted(operation_to_fn.keys())}"
parser.add_argument("operation", help=help_msg)
parser.add_argument("--det-port", default="8081", help="det master port")
parser.add_argument("--det-host", default="localhost", help="det master address eg localhost or 192.168.1.2")
parser.add_argument(
"--det-host",
default="localhost",
help="det master address eg localhost or 192.168.1.2",
)
parser.add_argument("--cypress-default-command-timeout", default="4000")
parser.add_argument("--cypress-args", help="other cypress arguments")
parser.add_argument("--log-level")
Expand Down
9 changes: 4 additions & 5 deletions webui/tests/bin/try-for-flake.sh
Original file line number Diff line number Diff line change
@@ -1,23 +1,22 @@
#!/bin/bash
#!/bin/bash -e

# if tests are re-runnable
re_runnable=${1:-false}
cmd_prefix=docker-

c=0

if $re_runnable; then
make pre-e2e-tests
./bin/e2e-tests.py pre-e2e-tests
while true; do
c=$((c+1))
echo "run #$c"
make ${cmd_prefix}run-e2e-tests || break
./bin/e2e-tests.py run-e2e-tests
done
else # run the whole suite
while true; do
c=$((c+1))
echo "run #$c"
make ${cmd_prefix}e2e-tests || break
make test
done
fi

Expand Down