Loop until a file is detected as modified

src/test/libraryfilesynctest.cpp

@@ -63,35 +63,42 @@ class TempFileSystem {
         // Rely on the OS to generate the time stamps instead of using
         // QFileDevice::setFileTime() with a time stamp generated by
         // QDateTime. The latter has proven to produce non-monotonic
-        // time stamps that cause spurious test failures.
-        ASSERT_TRUE(file.open(QIODevice::ReadWrite | QIODevice::ExistingOnly | QIODevice::Append));
-        const auto fileSize = file.size();
-        // Grow and shrink the file to bump the modification time stamp
-        ASSERT_TRUE(file.resize(fileSize + 1));
-        ASSERT_TRUE(file.flush());
-        ASSERT_TRUE(file.resize(fileSize));
-        file.close();
-        const auto newLastModified = fileLastModified();
-        ASSERT_TRUE(newLastModified.isValid());
+        // sequences of time stamps when mixed with the file system
+        // time stamps.
+        auto newLastModified = oldLastModified;
+        do {
+            ASSERT_TRUE(file.open(
+                    QIODevice::ReadWrite |
+                    QIODevice::ExistingOnly |
+                    QIODevice::Append));
+            const auto fileSize = file.size();
+            // Grow and shrink the file to bump the modification time stamp
+            ASSERT_TRUE(file.resize(fileSize + 1));
+            ASSERT_TRUE(file.flush());
+            ASSERT_TRUE(file.resize(fileSize));
+            file.close();
+            ASSERT_EQ(newLastModified, oldLastModified);
+            newLastModified = fileLastModified();
+            ASSERT_TRUE(newLastModified.isValid());
+            ASSERT_GE(newLastModified, oldLastModified);
+            sleepAfterFileLastModifiedUpdated();
+            // Looping is required to fix spurious CI test failures
+            // for Ubuntu 20.04 where the time stamps sometimes do not
+            // progress as expected, i.e. the new time stamp might equal
+            // the old time stamp even though the file was modified.
+        } while (newLastModified <= oldLastModified);
+        // The new modification time must be strictly greater than
+        // the old modification time to establish valid preconditions
+        // for the tests! Since those time stamps are generated as
+        // side effects by the OS we need these ugly quirks here.
         ASSERT_GT(newLastModified, oldLastModified);
-        sleepAfterFileLastModifiedUpdated();
     }
 
     void removeFile() const {
         ASSERT_TRUE(m_fileInfo.toQFile().remove());
     }
 
   private:
-    void createFile(const QFileInfo& testFile) {
-        ASSERT_TRUE(m_tempDir.isValid());
-        ASSERT_FALSE(m_fileInfo.exists());
-        mixxxtest::copyFile(
-                testFile.absoluteFilePath(),
-                m_fileInfo.location());
-        ASSERT_TRUE(m_fileInfo.exists());
-        sleepAfterFileLastModifiedUpdated();
-    }
-
     const QTemporaryDir m_tempDir;
     const mixxx::FileInfo m_fileInfo;
 };
@@ -452,10 +459,13 @@ class LibraryFileSyncStatusOutdatedTest : public LibraryFileSyncTest {
 
   protected:
     TrackPointer prepareTestTrack() const override {
-        // Touch the file's modification time stamp to simulate an external
-        // modification by a 3rd party app after the track has been loaded.
+        // Touch the file's modification time stamp to simulate an
+        // independent file modification by a 3rd party app.
         updateFileLastModified();
 
+        // The track is loaded directly from the database and will still
+        // have the previous synchronization time stamp. It should then
+        // be detected as outdated.
         return reloadTrackFromDatabaseAndVerifySourceSyncStatus(
                 mixxx::TrackRecord::SourceSyncStatus::Outdated);
     }