From 3f58a12da394244137ba85249dd13d09cb07e927 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 9 Jun 2020 19:00:34 +0200 Subject: quotest: make sure to send all events before leaving --- tests/quotest.cpp | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/tests/quotest.cpp b/tests/quotest.cpp index ae5ece30..fefd0182 100644 --- a/tests/quotest.cpp +++ b/tests/quotest.cpp @@ -639,18 +639,24 @@ void TestManager::conclude() // .then([this] { finalize(); }); // STL-style auto* room = testSuite->room(); auto txnId = room->postHtmlText(plainReport, htmlReport); - connect(room, &Room::messageSent, this, - [this, room, txnId](const QString& serverTxnId) { - if (txnId != serverTxnId) - return; + // Now just wait until all the pending events reach the server + connectUntil(room, &Room::messageSent, this, [this, room] { + const auto& pendingEvents = room->pendingEvents(); + if (std::any_of(pendingEvents.begin(), pendingEvents.end(), + [](const PendingEventItem& pe) { + return pe.deliveryStatus() + < EventStatus::ReachedServer; + })) + return false; - clog << "Leaving the room" << endl; - auto* job = room->leaveRoom(); - connect(job, &BaseJob::finished, this, [this, job] { - Q_ASSERT(job->status().good()); - finalize(); - }); - }); + clog << "Leaving the room" << endl; + auto* job = room->leaveRoom(); + connect(job, &BaseJob::finished, this, [this, job] { + Q_ASSERT(job->status().good()); + finalize(); + }); + return true; + }); } void TestManager::finalize() -- cgit v1.2.3 From 08f4490d4d770878f805d26bfe1d0ef9cb3f7393 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Wed, 10 Jun 2020 08:31:03 +0200 Subject: TestSuite::sendRedaction: fix a stale TODO Add another TODO instead :-| --- tests/quotest.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/quotest.cpp b/tests/quotest.cpp index fefd0182..79720812 100644 --- a/tests/quotest.cpp +++ b/tests/quotest.cpp @@ -357,10 +357,7 @@ TEST_IMPL(sendReaction) FAIL_TEST(); } - // TODO: Check that it came back as a reaction event and that it attached to - // the right event - connectUntil( - targetRoom, &Room::updatedEvent, this, + connectUntil(targetRoom, &Room::updatedEvent, this, [this, thisTest, txnId, key, targetEvtId](const QString& actualTargetEvtId) { if (actualTargetEvtId != targetEvtId) return false; @@ -376,6 +373,7 @@ TEST_IMPL(sendReaction) FINISH_TEST(is(*evt) && !evt->id().isEmpty() && evt->relation().key == key && evt->transactionId() == txnId); + // TODO: Test removing the reaction }); return false; } -- cgit v1.2.3 From f3b8dbe01a43c5334a371edda833173468d99dc4 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Wed, 10 Jun 2020 08:31:43 +0200 Subject: Room: fix messageSent() being emitted too early Closes #406. --- lib/room.cpp | 15 +++++++-------- tests/quotest.cpp | 30 ++++++++++++++++++++++-------- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/lib/room.cpp b/lib/room.cpp index 23e07cae..22ec1c82 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -1565,18 +1565,17 @@ QString Room::Private::doSendEvent(const RoomEvent* pEvent) std::bind(&Room::Private::onEventSendingFailure, this, txnId, call)); Room::connect(call, &BaseJob::success, q, [this, call, txnId] { - emit q->messageSent(txnId, call->eventId()); auto it = q->findPendingEvent(txnId); - if (it == unsyncedEvents.end()) { + if (it != unsyncedEvents.end()) { + if (it->deliveryStatus() != EventStatus::ReachedServer) { + it->setReachedServer(call->eventId()); + emit q->pendingEventChanged(int(it - unsyncedEvents.begin())); + } + } else qCDebug(EVENTS) << "Pending event for transaction" << txnId << "already merged"; - return; - } - if (it->deliveryStatus() != EventStatus::ReachedServer) { - it->setReachedServer(call->eventId()); - emit q->pendingEventChanged(int(it - unsyncedEvents.begin())); - } + emit q->messageSent(txnId, call->eventId()); }); } else onEventSendingFailure(txnId); diff --git a/tests/quotest.cpp b/tests/quotest.cpp index 79720812..e47bf651 100644 --- a/tests/quotest.cpp +++ b/tests/quotest.cpp @@ -493,18 +493,28 @@ TEST_IMPL(sendAndRedact) if (txnId.isEmpty()) FAIL_TEST(); - connect(targetRoom, &Room::messageSent, this, + connectUntil(targetRoom, &Room::messageSent, this, [this, thisTest, txnId](const QString& tId, const QString& evtId) { if (tId != txnId) - return; + return false; + + // The event may end up having been merged, and that's ok; + // but if it's not, it has to be in the ReachedServer state. + if (auto it = room()->findPendingEvent(tId); + it != room()->pendingEvents().cend() + && it->deliveryStatus() != EventStatus::ReachedServer) { + clog << "Incorrect sent event status (" + << it->deliveryStatus() << ')' << endl; + FAIL_TEST(); + } clog << "Redacting the message" << endl; targetRoom->redactEvent(evtId, origin); - connectUntil(targetRoom, &Room::addedMessages, this, [this, thisTest, evtId] { return checkRedactionOutcome(thisTest, evtId); }); + return false; }); return false; } @@ -640,12 +650,16 @@ void TestManager::conclude() // Now just wait until all the pending events reach the server connectUntil(room, &Room::messageSent, this, [this, room] { const auto& pendingEvents = room->pendingEvents(); - if (std::any_of(pendingEvents.begin(), pendingEvents.end(), - [](const PendingEventItem& pe) { - return pe.deliveryStatus() - < EventStatus::ReachedServer; - })) + if (auto c = std::count_if(pendingEvents.begin(), pendingEvents.end(), + [](const PendingEventItem& pe) { + return pe.deliveryStatus() + < EventStatus::ReachedServer; + }); + c > 0) { + clog << "Events to reach the server: " << c << ", not leaving yet" + << endl; return false; + } clog << "Leaving the room" << endl; auto* job = room->leaveRoom(); -- cgit v1.2.3 From 1293972514b10cd7a5ca6435646c496bb427e305 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Wed, 10 Jun 2020 11:33:36 +0200 Subject: libquotient.pri: set /std:c++17 explicitly Qt 5.9's qmake only seems to use CONFIG *= c++1z for GCC/LLVM but not for MSVC. --- libquotient.pri | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libquotient.pri b/libquotient.pri index 494a163a..68e7b590 100644 --- a/libquotient.pri +++ b/libquotient.pri @@ -7,9 +7,10 @@ CONFIG *= c++1z warn_on rtti_off create_prl object_parallel_to_source win32-msvc* { # Quotient code base does not play well with NMake inference rules CONFIG *= no_batch - QMAKE_CXXFLAGS_WARN_ON += -wd4100 -wd4267 + QMAKE_CXXFLAGS_WARN_ON *= -wd4100 -wd4267 + QMAKE_CXXFLAGS *= /std:c++17 # Older Qt doesn't understand c++1z above } else { - QMAKE_CXXFLAGS_WARN_ON += -Wno-unused-parameter + QMAKE_CXXFLAGS_WARN_ON *= -Wno-unused-parameter } contains(DEFINES, Quotient_E2EE_ENABLED=.) { -- cgit v1.2.3 From 4237a138f9ad1baa7384f3a539e9343a1471b538 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 9 Jun 2020 15:09:37 +0200 Subject: CI: test more configurations --- .appveyor.yml | 22 ++++++++++++++++------ .travis.yml | 43 ++++++++++++++++++++++++++++++------------- 2 files changed, 46 insertions(+), 19 deletions(-) diff --git a/.appveyor.yml b/.appveyor.yml index 13991ada..764d56d6 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -3,12 +3,22 @@ image: Visual Studio 2017 environment: CMAKE_ARGS: '-G "NMake Makefiles JOM" -DBUILD_SHARED_LIBS=OFF -DCMAKE_BUILD_TYPE=RelWithDebInfo' matrix: - - QTDIR: C:\Qt\5.13\msvc2017_64 - VCVARS: "vcvars64.bat" - PLATFORM: - - QTDIR: C:\Qt\5.13\msvc2017 + - QTDIR: C:\Qt\5.13\msvc2017 # Fresh Qt, 32-bit VCVARS: "vcvars32.bat" PLATFORM: x86 + - QTDIR: C:\Qt\5.13\msvc2017_64 # Fresh Qt, 64-bit + VCVARS: "vcvars64.bat" + PLATFORM: + - QTDIR: C:\Qt\5.9\msvc2017_64 # Oldest supported Qt, 64-bit, E2EE + VCVARS: "vcvars64.bat" + QMAKE_E2EE_ARGS: '"DEFINES += Quotient_E2EE_ENABLED USE_INTREE_LIBQOLM" "INCLUDEPATH += olm/include" "LIBS += -Lbuild/olm"' + CMAKE_E2EE_ARGS: '-DQuotient_ENABLE_E2EE=ON -DOlm_DIR=build/olm' + PLATFORM: + - QTDIR: C:\Qt\5.13\msvc2017_64 # Fresh Qt, 64-bit, E2EE + VCVARS: "vcvars64.bat" + QMAKE_E2EE_ARGS: '"DEFINES += Quotient_E2EE_ENABLED USE_INTREE_LIBQOLM" "INCLUDEPATH += olm/include" "LIBS += -Lbuild/olm"' + CMAKE_E2EE_ARGS: '-DQuotient_ENABLE_E2EE=ON -DOlm_DIR=build/olm' + PLATFORM: init: - call "%QTDIR%\bin\qtenv2.bat" @@ -23,10 +33,10 @@ before_build: - cmake --build build/olm build_script: -- cmake %CMAKE_ARGS% -H. -Bbuild "-DOlm_DIR=build/olm" +- cmake %CMAKE_ARGS% %CMAKE_E2EE_ARGS% -H. -Bbuild - cmake --build build # qmake uses olm just built by CMake - it can't build olm on its own. -- qmake "INCLUDEPATH += olm/include" "LIBS += -Lbuild" "LIBS += -Lbuild/olm" && jom +- qmake -Wall quotest.pro -- %QMAKE_E2EE_ARGS% && jom #after_build: #- cmake --build build --target install diff --git a/.travis.yml b/.travis.yml index d1a62c7f..9982be30 100644 --- a/.travis.yml +++ b/.travis.yml @@ -19,18 +19,15 @@ env: - CMAKE_ARGS="-DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_INSTALL_PREFIX=/usr -DCMAKE_PREFIX_PATH=$DESTDIR/usr" - VALGRIND="valgrind --tool=memcheck --leak-check=yes --gen-suppressions=all --suppressions=tests/.valgrind.supp $VALGRIND_OPTIONS" -matrix: +matrix: # TODO: consider parallel execution, this thing takes an hour to run include: - os: linux compiler: gcc - - os: linux - compiler: gcc - env: [ 'E2EE="-DQuotient_ENABLE_E2EE=ON"', 'UPDATE_API=1' ] - os: linux compiler: clang - os: osx osx_image: xcode10.1 - env: [ 'PATH=/usr/local/opt/qt/bin:$PATH', 'VALGRIND=' ] + env: [ 'E2EE=1', 'VALGRIND=' ] addons: homebrew: update: true @@ -41,15 +38,32 @@ matrix: cache: directories: - $HOME/Library/Caches/Homebrew + # Check a few more advanced configurations + - os: linux + compiler: gcc + env: [ E2EE=1, UPDATE_API=1 ] # Check UPDATE_API with one of fatter options + - os: linux + compiler: clang + env: [ E2EE=1 ] + - os: linux + compiler: gcc + env: [ E2EE=1 ] before_install: - if [ -f "$(which ninja)" ]; then export CMAKE_ARGS="$CMAKE_ARGS -GNinja"; fi +- if [ "$TRAVIS_OS_NAME" = "osx" ]; then export PATH=/usr/local/opt/qt/bin:$PATH; fi # The recent GTAD uses std::filesystem that's not available in stock bionic - | if [ -n "$UPDATE_API" ]; then export CC=gcc-8 CXX=g++-8 export CMAKE_UPDATE_API_ARGS="-DMATRIX_DOC_PATH=../matrix-doc -DGTAD_PATH=../gtad/gtad" fi +- | + if [ -n "$E2EE" ]; then + export CMAKE_E2EE_ARGS="-DQuotient_ENABLE_E2EE=ON" + export QMAKE_E2EE_ARGS='"DEFINES += Quotient_E2EE_ENABLED USE_INTREE_LIBQOLM" "INCLUDEPATH += olm/include" "LIBS += -Lolm/build"' + export LIB_PATH_E2EE=olm/build + fi # RPM spec-style: swallow a command with default parameters into an alias # and add/override parameters further in the code if/as necessary - shopt -s expand_aliases @@ -58,11 +72,14 @@ before_install: install: - pushd .. # Go out of libQuotient source tree -- git clone https://gitlab.matrix.org/matrix-org/olm.git -- pushd olm -- _cmake_config -- _cmake_build --target install -- popd +- | + if [ -n "$E2EE" ]; then + git clone https://gitlab.matrix.org/matrix-org/olm.git + pushd olm + _cmake_config + _cmake_build --target install + popd + fi - | if [ -n "$UPDATE_API" ]; then @@ -76,7 +93,7 @@ install: - popd # back to libQuotient source tree before_script: -- _cmake_config $CMAKE_UPDATE_API_ARGS $E2EE +- _cmake_config $CMAKE_UPDATE_API_ARGS $CMAKE_E2EE_ARGS - if [ -n "$UPDATE_API" ]; then _cmake_build --target update-api; fi script: @@ -85,10 +102,10 @@ script: - cmake $CMAKE_ARGS tests -Bbuild-test - cmake --build build-test --target all # Build with qmake -- qmake quotest.pro "CONFIG += debug" "CONFIG -= app_bundle" "QMAKE_CC = $CC" "QMAKE_CXX = $CXX" "INCLUDEPATH += olm/include" "LIBS += -Lbuild/lib" "LIBS += -Lolm/build" +- qmake -Wall quotest.pro "CONFIG += debug" "CONFIG -= app_bundle" "QMAKE_CC = $CC" "QMAKE_CXX = $CXX" -- $QMAKE_E2EE_ARGS - make all # Run the qmake-compiled quotest under valgrind -- if [ "$TEST_USER" != "" ]; then LD_LIBRARY_PATH="olm/build" $VALGRIND ./quotest "$TEST_USER" "$TEST_PWD" quotest-travis '#quotest:matrix.org' "Travis CI job $TRAVIS_JOB_NUMBER"; fi +- if [ "$TEST_USER" != "" ]; then LD_LIBRARY_PATH="$LIB_PATH_E2EE" $VALGRIND ./quotest "$TEST_USER" "$TEST_PWD" quotest-travis '#quotest:matrix.org' "Travis CI job $TRAVIS_JOB_NUMBER"; fi notifications: webhooks: -- cgit v1.2.3 From 0ad4e7da9f561b158cecf793ab34f2968b457ca8 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Wed, 10 Jun 2020 21:21:38 +0200 Subject: CMakeLists.txt: exclude key_backup.yaml from API As of now, it's a bit immature for code generation. Will be reincluded once it gets better. --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index c61c2682..8e56e856 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -211,6 +211,7 @@ if (MATRIX_DOC_PATH AND GTAD_PATH) ${FULL_CSAPI_SRC_DIR} old_sync.yaml- room_initial_sync.yaml- # deprecated search.yaml- # current GTAD is limited in handling move-only data + key_backup.yaml- # immature and buggy in terms of API definition sync.yaml- # we have a better handcrafted implementation WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}/lib SOURCES gtad/gtad.yaml -- cgit v1.2.3 From 9fc2c15dc021b2fddcb24bbc367c02639ec1256b Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Wed, 10 Jun 2020 21:27:17 +0200 Subject: syncdata.h: fix an old warning about operator<< The warning has been that it is declared as a friend but not exactly in namespace Quotient (though all compilers still could find and link it but only as long as it is defined in the namespace). Now instead of being declared as a friend it's just declared in the namespace :) --- lib/syncdata.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/syncdata.h b/lib/syncdata.h index 752dd485..67d04557 100644 --- a/lib/syncdata.h +++ b/lib/syncdata.h @@ -41,9 +41,9 @@ struct RoomSummary { /// Merge the contents of another RoomSummary object into this one /// \return true, if the current object has changed; false otherwise bool merge(const RoomSummary& other); - - friend QDebug operator<<(QDebug dbg, const RoomSummary& rs); }; +QDebug operator<<(QDebug dbg, const RoomSummary& rs); + template <> struct JsonObjectConverter { -- cgit v1.2.3