From 0f8d6cc1c1c6c3f85b1c4a382f9859ae3e99f62d Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Wed, 22 Jan 2020 13:31:22 +0000 Subject: [PATCH] Add a Sytest blacklist file (#849) --- docs/sytest.md | 19 +++++----- show-expected-fail-tests.sh | 72 +++++++++++++++++++++++++++++------- sytest-blacklist | 2 + testfile => sytest-whitelist | 0 4 files changed, 70 insertions(+), 23 deletions(-) create mode 100644 sytest-blacklist rename testfile => sytest-whitelist (100%) diff --git a/docs/sytest.md b/docs/sytest.md index e936dc49..6d03270b 100644 --- a/docs/sytest.md +++ b/docs/sytest.md @@ -2,16 +2,17 @@ Dendrite uses [SyTest](https://github.com/matrix-org/sytest) for its integration testing. When creating a new PR, add the test IDs (see below) that -your PR should allow to pass to `testfile` in dendrite's root directory. Not all -PRs need to make new tests pass. If we find your PR should be making a test pass -we may ask you to add to that file, as generally Dendrite's progress can be -tracked through the amount of SyTest tests it passes. +your PR should allow to pass to `sytest-whitelist` in dendrite's root +directory. Not all PRs need to make new tests pass. If we find your PR should +be making a test pass we may ask you to add to that file, as generally +Dendrite's progress can be tracked through the amount of SyTest tests it +passes. ## Finding out which tests to add We recommend you run the tests locally by manually setting up SyTest or using a SyTest docker image. After running the tests, a script will print the tests you -need to add to `testfile` for you. +need to add to `sytest-whitelist`. You should proceed after you see no build problems for dendrite after running: @@ -50,16 +51,16 @@ EOF Run the tests: ```sh -./run-tests.pl -I Dendrite::Monolith -d ../dendrite/bin -W ../dendrite/testfile -O tap --all | tee results.tap +./run-tests.pl -I Dendrite::Monolith -d ../dendrite/bin -W ../dendrite/sytest-whitelist -O tap --all | tee results.tap ``` where `tee` lets you see the results while they're being piped to the file. Once the tests are complete, run the helper script to see if you need to add -any newly passing test names to `testfile` in the project's root directory: +any newly passing test names to `sytest-whitelist` in the project's root directory: ```sh -../dendrite/show-expected-fail-tests.sh results.tap ../dendrite/testfile +../dendrite/show-expected-fail-tests.sh results.tap ../dendrite/sytest-whitelist ../dendrite/sytest-blacklist ``` If the script prints nothing/exits with 0, then you're good to go. @@ -75,4 +76,4 @@ docker run --rm -v /path/to/dendrite/:/src/ matrixdotorg/sytest-dendrite where `/path/to/dendrite/` should be replaced with the actual path to your dendrite source code. The output should tell you if you need to add any tests to -`testfile`. +`sytest-whitelist`. diff --git a/show-expected-fail-tests.sh b/show-expected-fail-tests.sh index 80b842ab..d3872ad5 100755 --- a/show-expected-fail-tests.sh +++ b/show-expected-fail-tests.sh @@ -1,45 +1,89 @@ #! /bin/bash +# +# Parses a results.tap file from SyTest output and a file containing test names (a test whitelist) +# and checks whether a test name that exists in the whitelist (that should pass), failed or not. +# +# An optional blacklist file can be added, also containing test names, where if a test name is +# present, the script will not error even if the test is in the whitelist file and failed +# +# For each of these files, lines starting with '#' are ignored. +# +# Usage ./show-expected-fail-tests.sh results.tap whitelist [blacklist] results_file=$1 -testfile=$2 +whitelist_file=$2 +blacklist_file=$3 fail_build=0 +if [ $# -lt 2 ]; then + echo "Usage: $0 results.tap whitelist [blacklist]" + exit 1 +fi + if [ ! -f "$results_file" ]; then - echo "ERROR: Specified results file ${results_file} doesn't exist." + echo "ERROR: Specified results file '${results_file}' doesn't exist." fail_build=1 fi -if [ ! -f "$testfile" ]; then - echo "ERROR: Specified testfile ${testfile} doesn't exist." +if [ ! -f "$whitelist_file" ]; then + echo "ERROR: Specified test whitelist '${whitelist_file}' doesn't exist." fail_build=1 fi +blacklisted_tests=() + +# Check if a blacklist file was provided +if [ $# -eq 3 ]; then + # Read test blacklist file + if [ ! -f "$blacklist_file" ]; then + echo "ERROR: Specified test blacklist file '${blacklist_file}' doesn't exist." + fail_build=1 + fi + + # Read each line, ignoring those that start with '#' + blacklisted_tests="" + search_non_comments=$(grep -v '^#' ${blacklist_file}) + while read -r line ; do + # Record the blacklisted test name + blacklisted_tests+=("${line}") + done <<< "${search_non_comments}" # This allows us to edit blacklisted_tests in the while loop +fi + [ "$fail_build" = 0 ] || exit 1 passed_but_expected_fail=$(grep ' # TODO passed but expected fail' ${results_file} | sed -E 's/^ok [0-9]+ (\(expected fail\) )?//' | sed -E 's/( \([0-9]+ subtests\))? # TODO passed but expected fail$//') tests_to_add="" -already_in_testfile="" +already_in_whitelist="" -while read -r test_id; do - [ "${test_id}" = "" ] && continue - grep "${test_id}" "${testfile}" > /dev/null 2>&1 +while read -r test_name; do + # Ignore empty lines + [ "${test_name}" = "" ] && continue + + grep "${test_name}" "${whitelist_file}" > /dev/null 2>&1 if [ "$?" != "0" ]; then - tests_to_add="${tests_to_add}${test_id}\n" + # Check if this test name is blacklisted + if printf '%s\n' "${blacklisted_tests[@]}" | grep -q -P "^${test_name}$"; then + # Don't notify about this test + continue + fi + + # Append this test_name to the existing list + tests_to_add="${tests_to_add}${test_name}\n" fail_build=1 else - already_in_testfile="${already_in_testfile}${test_id}\n" + already_in_whitelist="${already_in_whitelist}${test_name}\n" fi done <<< "${passed_but_expected_fail}" if [ -n "${tests_to_add}" ]; then - echo "ERROR: The following passed tests are not present in testfile. Please append them to the file:" + echo "ERROR: The following passed tests are not present in $2. Please append them to the file:" echo -e "${tests_to_add}" fi -if [ -n "${already_in_testfile}" ]; then - echo "WARN: Tests in testfile still marked as expected fail:" - echo -e "${already_in_testfile}" +if [ -n "${already_in_whitelist}" ]; then + echo "WARN: Tests in the whitelist still marked as expected fail:" + echo -e "${already_in_whitelist}" fi exit ${fail_build} diff --git a/sytest-blacklist b/sytest-blacklist new file mode 100644 index 00000000..e0fd2968 --- /dev/null +++ b/sytest-blacklist @@ -0,0 +1,2 @@ +# Blacklisted due to flakiness +Remote users can join room by alias diff --git a/testfile b/sytest-whitelist similarity index 100% rename from testfile rename to sytest-whitelist