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 1/2] 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 e936dc493..6d03270bb 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 80b842ab1..d3872ad59 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 000000000..e0fd29686 --- /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 From 43ecf8d1f909f4eb71bba93f6e7a57db59ec5941 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Wed, 22 Jan 2020 16:11:40 +0000 Subject: [PATCH 2/2] Add more passing tests to the testfile, add test blacklist file (#848) --- sytest-blacklist | 12 ++++++++++++ sytest-whitelist | 23 +++++++++++++---------- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/sytest-blacklist b/sytest-blacklist index e0fd29686..2ba23f930 100644 --- a/sytest-blacklist +++ b/sytest-blacklist @@ -1,2 +1,14 @@ # Blacklisted due to flakiness Remote users can join room by alias + +# Blacklisted due to flakiness +POST /login can log in as a user with just the local part of the id + +# Blacklisted due to flakiness +avatar_url updates affect room member events + +# Blacklisted due to flakiness +Room members can override their displayname on a room-specific basis + +# Blacklisted due to flakiness +Alias creators can delete alias with no ops diff --git a/sytest-whitelist b/sytest-whitelist index a869aeff1..0ba420d3b 100644 --- a/sytest-whitelist +++ b/sytest-whitelist @@ -17,10 +17,6 @@ POST /register rejects registration of usernames with 'é' POST /register rejects registration of usernames with '\n' POST /register rejects registration of usernames with ''' GET /login yields a set of flows -POST /login can log in as a user -POST /login can log in as a user with just the local part of the id -POST /login as non-existing user is rejected -POST /login wrong password is rejected GET /events initially GET /initialSync initially Version responds 200 OK with valid structure @@ -55,7 +51,6 @@ PUT power_levels should not explode if the old power levels were empty Both GET and PUT work POST /rooms/:room_id/read_markers can create read marker User signups are forbidden from starting with '_' -Can logout all devices Request to logout with invalid an access token is rejected Request to logout without an access token is rejected Room creation reports m.room.create to myself @@ -71,10 +66,8 @@ Can get rooms/{roomId}/members for a departed room (SPEC-216) 3pid invite join with wrong but valid signature are rejected 3pid invite join valid signature but revoked keys are rejected 3pid invite join valid signature but unreachable ID server are rejected -Room members can override their displayname on a room-specific basis Room members can join a room with an overridden displayname displayname updates affect room member events -avatar_url updates affect room member events Real non-joined user cannot call /events on shared room Real non-joined user cannot call /events on invited room Real non-joined user cannot call /events on joined room @@ -159,7 +152,6 @@ Typing events appear in gapped sync Inbound federation of state requires event_id as a mandatory paramater Inbound federation of state_ids requires event_id as a mandatory paramater POST /register returns the same device_id as that in the request -POST /login returns the same device_id as that in the request POST /createRoom with creation content User can create and send/receive messages in a room with version 1 POST /createRoom ignores attempts to set the room version via creation_content @@ -191,7 +183,6 @@ GET /directory/room/:room_alias yields room ID PUT /directory/room/:room_alias creates alias Room aliases can contain Unicode Creators can delete alias -Alias creators can delete alias with no ops Alias creators can delete canonical alias with no ops Regular users cannot create room aliases within the AS namespace Deleting a non-existent alias should return a 404 @@ -200,6 +191,18 @@ Outbound federation can query room alias directory After deactivating account, can't log in with an email Remote room alias queries can handle Unicode Newly joined room is included in an incremental sync after invite -Outbound federation can query v1 /send_join Inbound /v1/make_join rejects remote attempts to join local users to rooms Inbound federation rejects invites which are not signed by the sender +Local room members see posted message events +Fetching eventstream a second time doesn't yield the message again +Local non-members don't see posted message events +Remote room members also see posted message events +Lazy loading parameters in the filter are strictly boolean +remote user can join room with version 1 +remote user can join room with version 2 +remote user can join room with version 3 +remote user can join room with version 4 +remote user can join room with version 5 +Inbound federation can query room alias directory +Outbound federation can query v2 /send_join +Inbound federation can receive v2 /send_join