From 5f1b9a17ca7c3f60b9e27bdded8e67890683e793 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Tue, 3 Jan 2017 22:27:41 -0500 Subject: [PATCH] Fix testgui for Windows (PR #137) Introduces TemporaryFile class to act as a proxy to fix autoreload test cases failing on windows due to the behavior of QTemporaryFile. --- tests/gui/CMakeLists.txt | 2 +- tests/gui/TemporaryFile.cpp | 92 +++++++++++++++++++++++++++++++++++++ tests/gui/TemporaryFile.h | 64 ++++++++++++++++++++++++++ tests/gui/TestGui.cpp | 10 ++-- tests/gui/TestGui.h | 6 ++- 5 files changed, 167 insertions(+), 7 deletions(-) create mode 100644 tests/gui/TemporaryFile.cpp create mode 100644 tests/gui/TemporaryFile.h diff --git a/tests/gui/CMakeLists.txt b/tests/gui/CMakeLists.txt index a1ca914fd..4c0eebbe6 100644 --- a/tests/gui/CMakeLists.txt +++ b/tests/gui/CMakeLists.txt @@ -13,6 +13,6 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -add_unit_test(NAME testgui SOURCES TestGui.cpp LIBS ${TEST_LIBRARIES}) +add_unit_test(NAME testgui SOURCES TestGui.cpp TemporaryFile.cpp LIBS ${TEST_LIBRARIES}) add_unit_test(NAME testguipixmaps SOURCES TestGuiPixmaps.cpp LIBS ${TEST_LIBRARIES}) diff --git a/tests/gui/TemporaryFile.cpp b/tests/gui/TemporaryFile.cpp new file mode 100644 index 000000000..879a558a9 --- /dev/null +++ b/tests/gui/TemporaryFile.cpp @@ -0,0 +1,92 @@ +/* + * Copyright (C) 2016 Danny Su + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "TemporaryFile.h" + +#include + +#ifdef Q_OS_WIN +const QString TemporaryFile::SUFFIX = ".win"; + +TemporaryFile::~TemporaryFile() +{ + if (m_tempFile.autoRemove()) { + m_file.remove(); + } +} +#endif + +bool TemporaryFile::open() +{ +#ifdef Q_OS_WIN + // Still call QTemporaryFile::open() so that it figures out the temporary + // file name to use. Assuming that by appending the SUFFIX to whatever + // QTemporaryFile chooses is also an available file. + bool tempFileOpened = m_tempFile.open(); + if (tempFileOpened) { + m_file.setFileName(filePath()); + return m_file.open(QIODevice::WriteOnly); + } + return false; +#else + return m_tempFile.open(); +#endif +} + +void TemporaryFile::close() +{ + m_tempFile.close(); +#ifdef Q_OS_WIN + m_file.close(); +#endif +} + +qint64 TemporaryFile::write(const char *data, qint64 maxSize) +{ +#ifdef Q_OS_WIN + return m_file.write(data, maxSize); +#else + return m_tempFile.write(data, maxSize); +#endif +} + +qint64 TemporaryFile::write(const QByteArray &byteArray) +{ +#ifdef Q_OS_WIN + return m_file.write(byteArray); +#else + return m_tempFile.write(byteArray); +#endif +} + +QString TemporaryFile::fileName() const +{ +#ifdef Q_OS_WIN + return QFileInfo(m_tempFile).fileName() + TemporaryFile::SUFFIX; +#else + return QFileInfo(m_tempFile).fileName(); +#endif +} + +QString TemporaryFile::filePath() const +{ +#ifdef Q_OS_WIN + return m_tempFile.fileName() + TemporaryFile::SUFFIX; +#else + return m_tempFile.fileName(); +#endif +} diff --git a/tests/gui/TemporaryFile.h b/tests/gui/TemporaryFile.h new file mode 100644 index 000000000..b16e2161a --- /dev/null +++ b/tests/gui/TemporaryFile.h @@ -0,0 +1,64 @@ +/* + * Copyright (C) 2016 Danny Su + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef KEEPASSX_TEMPORARYFILE_H +#define KEEPASSX_TEMPORARYFILE_H + +#include +#include +#include + +/** + * QTemporaryFile::close() doesn't actually close the file according to + * http://doc.qt.io/qt-5/qtemporaryfile.html: "For as long as the + * QTemporaryFile object itself is not destroyed, the unique temporary file + * will exist and be kept open internally by QTemporaryFile." + * + * This behavior causes issues when running tests on Windows. If the file is + * not closed, the testSave test will fail due to Access Denied. The + * auto-reload test also fails from Windows not triggering file change + * notification because the file isn't actually closed by QTemporaryFile. + * + * This class isolates the Windows specific logic that uses QFile to really + * close the test file when requested to. + */ +class TemporaryFile : public QObject +{ + Q_OBJECT + +public: +#ifdef Q_OS_WIN + ~TemporaryFile(); +#endif + + bool open(); + void close(); + qint64 write(const char *data, qint64 maxSize); + qint64 write(const QByteArray &byteArray); + + QString fileName() const; + QString filePath() const; + +private: + QTemporaryFile m_tempFile; +#ifdef Q_OS_WIN + QFile m_file; + static const QString SUFFIX; +#endif +}; + +#endif // KEEPASSX_TEMPORARYFILE_H diff --git a/tests/gui/TestGui.cpp b/tests/gui/TestGui.cpp index 91271c189..01f165243 100644 --- a/tests/gui/TestGui.cpp +++ b/tests/gui/TestGui.cpp @@ -81,9 +81,10 @@ void TestGui::init() QCOMPARE(m_dbFile.write(m_dbData), static_cast((m_dbData.size()))); m_dbFile.close(); - m_dbFileName = QFileInfo(m_dbFile).fileName(); + m_dbFileName = m_dbFile.fileName(); + m_dbFilePath = m_dbFile.filePath(); - fileDialog()->setNextFileName(m_dbFile.fileName()); + fileDialog()->setNextFileName(m_dbFilePath); triggerAction("actionDatabaseOpen"); QWidget* databaseOpenWidget = m_mainWindow->findChild("databaseOpenWidget"); @@ -605,7 +606,7 @@ void TestGui::testDragAndDropGroup() void TestGui::testSaveAs() { - QFileInfo fileInfo(m_dbFile.fileName()); + QFileInfo fileInfo(m_dbFilePath); QDateTime lastModified = fileInfo.lastModified(); m_db->metadata()->setName("SaveAs"); @@ -642,6 +643,7 @@ void TestGui::testSave() void TestGui::testDatabaseSettings() { + m_db->metadata()->setName("Save"); triggerAction("actionChangeDatabaseSettings"); QWidget* dbSettingsWidget = m_dbWidget->findChild("databaseSettingsWidget"); QSpinBox* transformRoundsSpinBox = dbSettingsWidget->findChild("transformRoundsSpinBox"); @@ -707,7 +709,7 @@ void TestGui::cleanupTestCase() void TestGui::checkDatabase(QString dbFileName) { if (dbFileName.isEmpty()) - dbFileName = m_dbFile.fileName(); + dbFileName = m_dbFilePath; CompositeKey key; key.addKey(PasswordKey("a")); diff --git a/tests/gui/TestGui.h b/tests/gui/TestGui.h index ab5bf5f2f..c2e0e372e 100644 --- a/tests/gui/TestGui.h +++ b/tests/gui/TestGui.h @@ -18,9 +18,10 @@ #ifndef KEEPASSX_TESTGUI_H #define KEEPASSX_TESTGUI_H +#include "TemporaryFile.h" + #include #include -#include class Database; class DatabaseTabWidget; @@ -67,8 +68,9 @@ private: DatabaseTabWidget* m_tabWidget; DatabaseWidget* m_dbWidget; QByteArray m_dbData; - QTemporaryFile m_dbFile; + TemporaryFile m_dbFile; QString m_dbFileName; + QString m_dbFilePath; Database* m_db; };