Merge branch 'kitsune-expand-ci'

7 files changed, 96 insertions, 50 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:
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
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/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<RoomSummary> {
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=.) {
diff --git a/tests/quotest.cpp b/tests/quotest.cpp
index ae5ece30..e47bf651 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<ReactionEvent>(*evt) && !evt->id().isEmpty()
                         && evt->relation().key == key
                         && evt->transactionId() == txnId);
+            // TODO: Test removing the reaction
         });
     return false;
 }
@@ -495,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;
 }
@@ -639,18 +647,28 @@ 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;
-
-                clog << "Leaving the room" << endl;
-                auto* job = room->leaveRoom();
-                connect(job, &BaseJob::finished, this, [this, job] {
-                    Q_ASSERT(job->status().good());
-                    finalize();
-                });
-            });
+    // Now just wait until all the pending events reach the server
+    connectUntil(room, &Room::messageSent, this, [this, room] {
+        const auto& pendingEvents = room->pendingEvents();
+        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();
+        connect(job, &BaseJob::finished, this, [this, job] {
+            Q_ASSERT(job->status().good());
+            finalize();
+        });
+        return true;
+    });
 }
 
 void TestManager::finalize()