Improve Database and CLI tests

keepassxreboot · Oct 11, 2019 · b8ecef7 · b8ecef7
1 parent d5408ec
commit b8ecef7
Show file tree

Hide file tree

Showing 10 changed files with 130 additions and 80 deletions.
diff --git a/src/core/Database.cpp b/src/core/Database.cpp
@@ -53,7 +53,7 @@ Database::Database()
 
     s_uuidMap.insert(m_uuid, this);
 
-    connect(m_metadata, SIGNAL(metadataModified()), this, SLOT(markAsModified()));
+    connect(m_metadata, SIGNAL(metadataModified()), SLOT(markAsModified()));
     connect(m_timer, SIGNAL(timeout()), SIGNAL(databaseModified()));
     connect(this, SIGNAL(databaseOpened()), SLOT(updateCommonUsernames()));
     connect(this, SIGNAL(databaseSaved()), SLOT(updateCommonUsernames()));
@@ -188,7 +188,7 @@ bool Database::save(QString* error, bool atomic, bool backup)
 /**
  * Save the database to a specific file.
  *
- * If atmoic is false, this function uses QTemporaryFile instead of QSaveFile
+ * If atomic is false, this function uses QTemporaryFile instead of QSaveFile
  * due to a bug in Qt (https://bugreports.qt.io/browse/QTBUG-57299) that may
  * prevent the QSaveFile from renaming itself when using Dropbox, Google Drive,
  * or OneDrive.

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -224,7 +224,7 @@ if(WITH_XC_KEESHARE)
 endif()
 
 add_unit_test(NAME testdatabase SOURCES TestDatabase.cpp
-        LIBS ${TEST_LIBRARIES})
+        LIBS testsupport ${TEST_LIBRARIES})
 
 add_unit_test(NAME testtools SOURCES TestTools.cpp
         LIBS ${TEST_LIBRARIES})

diff --git a/tests/TestCli.cpp b/tests/TestCli.cpp
@@ -132,36 +132,32 @@ void TestCli::init()
 
     m_stdinFile.reset(new TemporaryFile());
     m_stdinFile->open();
-    m_stdinHandle = fdopen(m_stdinFile->handle(), "r+");
-    Utils::STDIN = m_stdinHandle;
+    Utils::STDIN = fdopen(m_stdinFile->handle(), "r+");
 
     m_stdoutFile.reset(new TemporaryFile());
     m_stdoutFile->open();
-    m_stdoutHandle = fdopen(m_stdoutFile->handle(), "r+");
-    Utils::STDOUT = m_stdoutHandle;
+    Utils::STDOUT = fdopen(m_stdoutFile->handle(), "r+");
 
     m_stderrFile.reset(new TemporaryFile());
     m_stderrFile->open();
-    m_stderrHandle = fdopen(m_stderrFile->handle(), "r+");
-    Utils::STDERR = m_stderrHandle;
+    Utils::STDERR = fdopen(m_stderrFile->handle(), "r+");
 }
 
 void TestCli::cleanup()
 {
     m_dbFile.reset();
-
     m_dbFile2.reset();
+    m_keyFileProtectedDbFile.reset();
+    m_keyFileProtectedNoPasswordDbFile.reset();
+    m_yubiKeyProtectedDbFile.reset();
 
     m_stdinFile.reset();
-    m_stdinHandle = stdin;
     Utils::STDIN = stdin;
 
     m_stdoutFile.reset();
     Utils::STDOUT = stdout;
-    m_stdoutHandle = stdout;
 
     m_stderrFile.reset();
-    m_stderrHandle = stderr;
     Utils::STDERR = stderr;
 }
 
@@ -172,8 +168,8 @@ void TestCli::cleanupTestCase()
 QSharedPointer<Database> TestCli::readTestDatabase() const
 {
     Utils::Test::setNextPassword("a");
-    auto db = QSharedPointer<Database>(Utils::unlockDatabase(m_dbFile->fileName(), true, "", "", m_stdoutHandle));
-    m_stdoutFile->seek(ftell(m_stdoutHandle)); // re-synchronize handles
+    auto db = QSharedPointer<Database>(Utils::unlockDatabase(m_dbFile->fileName(), true, "", "", Utils::STDOUT));
+    m_stdoutFile->seek(ftell(Utils::STDOUT)); // re-synchronize handles
     return db;
 }
 
@@ -269,17 +265,25 @@ void TestCli::testAdd()
     addCmd.execute({"add", "-q", "-u", "newuser", "-g", "-L", "20", m_dbFile->fileName(), "/newentry-quiet"});
     m_stdoutFile->seek(pos);
     m_stderrFile->seek(posErr);
-    QCOMPARE(m_stdoutFile->readAll(), QByteArray(""));
     QCOMPARE(m_stderrFile->readAll(), QByteArray(""));
+    QCOMPARE(m_stdoutFile->readAll(), QByteArray(""));
     db = readTestDatabase();
     entry = db->rootGroup()->findEntryByPath("/newentry-quiet");
     QVERIFY(entry);
     QCOMPARE(entry->password().size(), 20);
 
+    pos = m_stdoutFile->pos();
+    posErr = m_stderrFile->pos();
     Utils::Test::setNextPassword("a");
     Utils::Test::setNextPassword("newpassword");
     addCmd.execute(
         {"add", "-u", "newuser2", "--url", "https://example.net/", "-p", m_dbFile->fileName(), "/newuser-entry2"});
+    m_stdoutFile->seek(pos);
+    m_stderrFile->seek(posErr);
+    m_stdoutFile->readLine(); // skip password prompt
+    m_stdoutFile->readLine(); // skip password input
+    QCOMPARE(m_stderrFile->readAll(), QByteArray(""));
+    QCOMPARE(m_stdoutFile->readAll(), QByteArray("Successfully added entry newuser-entry2.\n"));
 
     db = readTestDatabase();
     entry = db->rootGroup()->findEntryByPath("/newuser-entry2");
@@ -296,8 +300,8 @@ void TestCli::testAdd()
     m_stdoutFile->seek(pos);
     m_stderrFile->seek(posErr);
     m_stdoutFile->readLine(); // skip password prompt
-    QCOMPARE(m_stdoutFile->readAll(), QByteArray("Successfully added entry newuser-entry3.\n"));
     QCOMPARE(m_stderrFile->readAll(), QByteArray(""));
+    QCOMPARE(m_stdoutFile->readAll(), QByteArray("Successfully added entry newuser-entry3.\n"));
 
     db = readTestDatabase();
     entry = db->rootGroup()->findEntryByPath("/newuser-entry3");
@@ -326,8 +330,8 @@ void TestCli::testAdd()
     m_stdoutFile->seek(pos);
     m_stderrFile->seek(posErr);
     m_stdoutFile->readLine(); // skip password prompt
-    QCOMPARE(m_stdoutFile->readAll(), QByteArray("Successfully added entry newuser-entry4.\n"));
     QCOMPARE(m_stderrFile->readAll(), QByteArray(""));
+    QCOMPARE(m_stdoutFile->readAll(), QByteArray("Successfully added entry newuser-entry4.\n"));
 
     db = readTestDatabase();
     entry = db->rootGroup()->findEntryByPath("/newuser-entry4");
@@ -528,7 +532,7 @@ void TestCli::testCreate()
 
     QScopedPointer<QTemporaryDir> testDir(new QTemporaryDir());
 
-    QString databaseFilename = testDir->path() + "testCreate1.kdbx";
+    QString databaseFilename = testDir->path() + "/testCreate1.kdbx";
     // Password
     Utils::Test::setNextPassword("a");
     createCmd.execute({"create", databaseFilename});
@@ -556,8 +560,8 @@ void TestCli::testCreate()
     QCOMPARE(m_stderrFile->readAll(), errorMessage.toUtf8());
 
     // Testing with keyfile creation
-    QString databaseFilename2 = testDir->path() + "testCreate2.kdbx";
-    QString keyfilePath = testDir->path() + "keyfile.txt";
+    QString databaseFilename2 = testDir->path() + "/testCreate2.kdbx";
+    QString keyfilePath = testDir->path() + "/keyfile.txt";
     pos = m_stdoutFile->pos();
     errPos = m_stderrFile->pos();
     Utils::Test::setNextPassword("a");
@@ -575,7 +579,7 @@ void TestCli::testCreate()
     QVERIFY(db2);
 
     // Testing with existing keyfile
-    QString databaseFilename3 = testDir->path() + "testCreate3.kdbx";
+    QString databaseFilename3 = testDir->path() + "/testCreate3.kdbx";
     pos = m_stdoutFile->pos();
     errPos = m_stderrFile->pos();
     Utils::Test::setNextPassword("a");

diff --git a/tests/TestCli.h b/tests/TestCli.h
@@ -88,9 +88,6 @@ private slots:
     QScopedPointer<TemporaryFile> m_stdoutFile;
     QScopedPointer<TemporaryFile> m_stderrFile;
     QScopedPointer<TemporaryFile> m_stdinFile;
-    FILE* m_stdoutHandle = stdout;
-    FILE* m_stderrHandle = stderr;
-    FILE* m_stdinHandle = stdin;
 };
 
 #endif // KEEPASSXC_TESTCLI_H
diff --git a/tests/TestDatabase.cpp b/tests/TestDatabase.cpp
@@ -20,17 +20,18 @@
 #include "TestGlobal.h"
 
 #include <QSignalSpy>
-#include <QTemporaryFile>
 
 #include "config-keepassx-tests.h"
 #include "core/Metadata.h"
-#include "core/Tools.h"
 #include "crypto/Crypto.h"
 #include "format/KeePass2Writer.h"
 #include "keys/PasswordKey.h"
+#include "util/TemporaryFile.h"
 
 QTEST_GUILESS_MAIN(TestDatabase)
 
+static QString dbFileName = QStringLiteral(KEEPASSX_TEST_DATA_DIR).append("/NewDatabase.kdbx");
+
 void TestDatabase::initTestCase()
 {
     QVERIFY(Crypto::init());
@@ -45,7 +46,7 @@ void TestDatabase::testOpen()
     auto key = QSharedPointer<CompositeKey>::create();
     key->addKey(QSharedPointer<PasswordKey>::create("a"));
 
-    bool ok = db->open(QString(KEEPASSX_TEST_DATA_DIR).append("/NewDatabase.kdbx"), key);
+    bool ok = db->open(dbFileName, key);
     QVERIFY(ok);
 
     QVERIFY(db->isInitialized());
@@ -57,16 +58,8 @@ void TestDatabase::testOpen()
 
 void TestDatabase::testSave()
 {
-    QByteArray data;
-    QFile sourceDbFile(QString(KEEPASSX_TEST_DATA_DIR).append("/NewDatabase.kdbx"));
-    QVERIFY(sourceDbFile.open(QIODevice::ReadOnly));
-    QVERIFY(Tools::readAllFromDevice(&sourceDbFile, data));
-    sourceDbFile.close();
-
-    QTemporaryFile tempFile;
-    QVERIFY(tempFile.open());
-    QCOMPARE(tempFile.write(data), static_cast<qint64>((data.size())));
-    tempFile.close();
+    TemporaryFile tempFile;
+    QVERIFY(tempFile.copyFromFile(dbFileName));
 
     auto db = QSharedPointer<Database>::create();
     auto key = QSharedPointer<CompositeKey>::create();
@@ -79,18 +72,63 @@ void TestDatabase::testSave()
     // Test safe saves
     db->metadata()->setName("test");
     QVERIFY(db->isModified());
+    QVERIFY2(db->save(&error), error.toLatin1());
+    QVERIFY(!db->isModified());
 
-    // TODO: cannot overwrite temp file...
     // Test unsafe saves
+    db->metadata()->setName("test2");
     QVERIFY2(db->save(&error, false, false), error.toLatin1());
-
-    QVERIFY2(db->save(&error), error.toLatin1());
     QVERIFY(!db->isModified());
 
     // Test save backups
+    db->metadata()->setName("test3");
     QVERIFY2(db->save(&error, true, true), error.toLatin1());
+    QVERIFY(!db->isModified());
+
+    // Confirm backup exists and then delete it
+    auto re = QRegularExpression("(\\.[^.]+)$");
+    auto match = re.match(tempFile.fileName());
+    auto backupFilePath = tempFile.fileName();
+    backupFilePath = backupFilePath.replace(re, "") + ".old" + match.captured(1);
+    QVERIFY(QFile::exists(backupFilePath));
+    QFile::remove(backupFilePath);
+    QVERIFY(!QFile::exists(backupFilePath));
+}
+
+void TestDatabase::testSignals()
+{
+    TemporaryFile tempFile;
+    QVERIFY(tempFile.copyFromFile(dbFileName));
+
+    auto db = QSharedPointer<Database>::create();
+    auto key = QSharedPointer<CompositeKey>::create();
+    key->addKey(QSharedPointer<PasswordKey>::create("a"));
+
+    QSignalSpy spyFilePathChanged(db.data(), SIGNAL(filePathChanged(const QString&, const QString&)));
+    QString error;
+    bool ok = db->open(tempFile.fileName(), key, &error);
+    QVERIFY(ok);
+    QCOMPARE(spyFilePathChanged.count(), 1);
+
+    QSignalSpy spyModified(db.data(), SIGNAL(databaseModified()));
+    db->metadata()->setName("test1");
+    QTRY_COMPARE(spyModified.count(), 1);
+
+    QSignalSpy spySaved(db.data(), SIGNAL(databaseSaved()));
+    QVERIFY(db->save(&error));
+    QCOMPARE(spySaved.count(), 1);
+
+    QSignalSpy spyFileChanged(db.data(), SIGNAL(databaseFileChanged()));
+    QVERIFY(tempFile.copyFromFile(dbFileName));
+    QTRY_COMPARE(spyFileChanged.count(), 1);
+    QTRY_VERIFY(!db->isModified());
 
+    db->metadata()->setName("test2");
+    QTRY_VERIFY(db->isModified());
 
+    QSignalSpy spyDiscarded(db.data(), SIGNAL(databaseDiscarded()));
+    QVERIFY(db->open(tempFile.fileName(), key, &error));
+    QCOMPARE(spyDiscarded.count(), 1);
 }
 
 void TestDatabase::testEmptyRecycleBinOnDisabled()

diff --git a/tests/TestDatabase.h b/tests/TestDatabase.h
@@ -29,6 +29,7 @@ private slots:
     void initTestCase();
     void testOpen();
     void testSave();
+    void testSignals();
     void testEmptyRecycleBinOnDisabled();
     void testEmptyRecycleBinOnNotCreated();
     void testEmptyRecycleBinOnEmpty();