Separate GuiFont + GuiLinespace tst_shell UTs

test/CMakeLists.txt

@@ -59,8 +59,10 @@ add_xtest(tst_neovimconnector)
 add_xtest(tst_callallmethods)
 add_xtest(tst_encoding)
 add_xtest(tst_msgpackiodevice)
-add_xtest_gui(tst_shell ${SRC_SHELL_PLATFORM})
 add_xtest_gui(tst_main)
+add_xtest_gui(tst_shell
+	${SRC_SHELL_PLATFORM}
+	mock_qsettings.cpp)
 add_xtest_gui(tst_qsettings
 	${SRC_SHELL_PLATFORM}
 	mock_qsettings.cpp)

test/common_gui.cpp

@@ -1,6 +1,7 @@
 #include "common_gui.h"
 
 #include <gui/mainwindow.h>
+#include <QFontDatabase>
 
 #include "common.h"
 
@@ -77,4 +78,18 @@ std::pair<NeovimConnector*, MainWindow*> CreateMainWindowWithRuntime() noexcept
 	return { c, w };
 }
 
+void LoadDejaVuSansMonoTestFonts() noexcept
+{
+	const QStringList fonts{
+		QStringLiteral("third-party/DejaVuSansMono.ttf"),
+		QStringLiteral("third-party/DejaVuSansMono-Bold.ttf"),
+		QStringLiteral("third-party/DejaVuSansMono-BoldOblique.ttf") };
+
+	for (const auto& path : fonts) {
+		QString abs_path_to_font(CMAKE_SOURCE_DIR);
+		abs_path_to_font.append("/").append(path);
+		QFontDatabase::addApplicationFont(abs_path_to_font);
+	}
+}
+
 } // namespace NeovimQt

test/common_gui.h

@@ -13,4 +13,6 @@ std::pair<NeovimConnector*, MainWindow*> CreateMainWindow() noexcept;
 
 std::pair<NeovimConnector*, MainWindow*> CreateMainWindowWithRuntime() noexcept;
 
+void LoadDejaVuSansMonoTestFonts() noexcept;
+
 } // namespace NeovimQt

test/tst_qsettings.cpp

@@ -37,6 +37,7 @@ static void SendNeovimCommand(NeovimConnector& connector, const QString& command
 void TestQSettings::initTestCase() noexcept
 {
 	NeovimQt::MockQSettings::EnableByDefault();
+	LoadDejaVuSansMonoTestFonts();
 }
 
 void TestQSettings::cleanup() noexcept
@@ -99,7 +100,8 @@ void TestQSettings::GuiFont() noexcept
 
 	QSettings settings;
 
-	const QString fontDesc{ QStringLiteral("%1:h20").arg(GetPlatformTestFont()) };
+	const QString cmdFontSize14{ QStringLiteral("GuiFont! :h14") };
+	const QString fontDesc{ QStringLiteral("DejaVu Sans Mono:h20") };
 	const QString fontCommand{ QStringLiteral("GuiFont! %1").arg(fontDesc) };
 
 	SendNeovimCommand(connector, fontCommand);

test/tst_shell.cpp

@@ -10,6 +10,7 @@
 
 #include "common.h"
 #include "common_gui.h"
+#include "mock_qsettings.h"
 
 #if defined(Q_OS_WIN) && defined(USE_STATIC_QT)
 #include <QtPlugin>
@@ -24,11 +25,14 @@ class TestShell : public QObject
 
 private slots:
 	void initTestCase() noexcept;
+	void cleanup() noexcept;
+
 	void benchStart() noexcept;
 	void startVarsShellWidget() noexcept;
 	void startVarsMainWindow() noexcept;
 	void gviminit() noexcept;
-	void guiShimCommands() noexcept;
+	void GuiLinespaceCommand() noexcept;
+	void GuiFontCommand() noexcept;
 	void CloseEvent_data() noexcept;
 	void CloseEvent() noexcept;
 	void GetClipboard_data() noexcept;
@@ -48,18 +52,16 @@ static void SignalPrintError(QString msg, const QVariant& err) noexcept
 
 void TestShell::initTestCase() noexcept
 {
-	const QStringList fonts{
-		QStringLiteral("third-party/DejaVuSansMono.ttf"),
-		QStringLiteral("third-party/DejaVuSansMono-Bold.ttf"),
-		QStringLiteral("third-party/DejaVuSansMono-BoldOblique.ttf") };
-
-	for (const auto& path : fonts) {
-		QString abs_path_to_font(CMAKE_SOURCE_DIR);
-		abs_path_to_font.append("/").append(path);
-		QFontDatabase::addApplicationFont(abs_path_to_font);
-	}
+	NeovimQt::MockQSettings::EnableByDefault();
+	LoadDejaVuSansMonoTestFonts();
 }
 
+void TestShell::cleanup() noexcept
+{
+	NeovimQt::MockQSettings::ClearAllContents();
+}
+
+
 void TestShell::benchStart() noexcept
 {
 	QBENCHMARK
@@ -101,54 +103,84 @@ void TestShell::gviminit() noexcept
 	QCOMPARE(cmd.at(0).at(2).toByteArray(), QByteArray("1"));
 }
 
-void TestShell::guiShimCommands() noexcept
+void TestShell::GuiLinespaceCommand() noexcept
 {
 	auto cw{ CreateMainWindowWithRuntime() };
 	NeovimConnector* c{ cw.first };
 	MainWindow* w{ cw.second };
 
 	QObject::connect(c->neovimObject(), &NeovimApi1::err_vim_command_output, SignalPrintError);
 
-	QSignalSpy cmd_font(
-		c->neovimObject()->vim_command_output(c->encode("GuiFont!")), &MsgpackRequest::finished);
-	QVERIFY(cmd_font.isValid());
-	QVERIFY2(SPYWAIT(cmd_font), "Waiting for GuiFont");
+	QSignalSpy cmd_ls{ c->neovimObject()->vim_command_output(c->encode("GuiLinespace")),
+		&MsgpackRequest::finished };
 
-	QSignalSpy cmd_ls(
-		c->neovimObject()->vim_command_output(c->encode("GuiLinespace")),
-		&MsgpackRequest::finished);
 	QVERIFY(cmd_ls.isValid());
 	QVERIFY2(SPYWAIT(cmd_ls), "Waiting for GuiLinespace");
 
-	// Test font attributes
-	const QString cmdFontSize14{ QStringLiteral("GuiFont! %1:h14").arg(GetPlatformTestFont()) };
-	const QString expectedFontSize14{ QStringLiteral("%1:h14").arg(GetPlatformTestFont()) };
-	QSignalSpy cmd_gf{ c->neovimObject()->vim_command_output(c->encode(cmdFontSize14)),
+	QSignalSpy spyFontChanged{ w->shell(), &ShellWidget::shellFontChanged };
+	QSignalSpy spyLineSpace2{ c->neovimObject()->vim_command_output(c->encode("GuiLinespace 2")),
+		&MsgpackRequest::finished };
+
+	QVERIFY(spyFontChanged.isValid());
+	QVERIFY(SPYWAIT(spyFontChanged));
+
+	QSignalSpy spyLineSpaceValue{ c->neovimObject()->vim_command_output(c->encode("GuiLinespace")),
+		&MsgpackRequest::finished };
+
+	QVERIFY(spyLineSpaceValue.isValid());
+	QVERIFY(SPYWAIT(spyLineSpaceValue));
+
+	QByteArray varObserved{ spyLineSpaceValue.at(0).at(2).toByteArray() };
+	QCOMPARE(varObserved, QByteArray{ "2" });
+}
+
+void TestShell::GuiFontCommand() noexcept
+{
+	auto cw{ CreateMainWindowWithRuntime() };
+	NeovimConnector* c{ cw.first };
+	MainWindow* w{ cw.second };
+
+	QObject::connect(c->neovimObject(), &NeovimApi1::err_vim_command_output, SignalPrintError);
+
+	QSignalSpy spyGuiFontNoArgs{ c->neovimObject()->vim_command_output(c->encode("GuiFont")),
 		&MsgpackRequest::finished };
-	QVERIFY(cmd_gf.isValid());
-	QVERIFY(SPYWAIT(cmd_gf));
+	QVERIFY(spyGuiFontNoArgs.isValid());
+	QVERIFY2(SPYWAIT(spyGuiFontNoArgs), "Waiting for GuiFont");
 
-	QSignalSpy spy_fontchange(w->shell(), &ShellWidget::shellFontChanged);
+	// Test font attributes
+	const QString cmdFontSize14{ QStringLiteral("GuiFont! DejaVu Sans Mono:h14") };
+	const QString expectedFontSize14{ QStringLiteral("DejaVu Sans Mono:h14") };
 
-	// Test Performance: timeout occurs often, set value carefully.
-	SPYWAIT(spy_fontchange, 2500 /*msec*/);
+	// Issue#977: concurrency issue inside of Shell::updateGuiFontRegisters() for guifont.
+	// A call to :GuiFont triggers a call update guifont=. If a second call to :GuiFont is made
+	// while the guifont= callback is still outstanding, the guifont value is mangled. This can be
+	// observed if the test delay below is removed.
+	QTest::qWait(500);
 
+	QSignalSpy spyGuiFontSize14{ c->neovimObject()->vim_command_output(c->encode(cmdFontSize14)),
+		&MsgpackRequest::finished };
+	QSignalSpy spyFontChangedSize14{ w->shell(), &ShellWidget::shellFontChanged };
+
+	QVERIFY(spyGuiFontSize14.isValid());
+	QVERIFY(SPYWAIT(spyGuiFontSize14));
+	QVERIFY(SPYWAIT(spyFontChangedSize14));
 	QCOMPARE(w->shell()->fontDesc(), expectedFontSize14);
 
 	// Normalization removes the :b attribute
 	const QString cmdFontBoldRemoved{
-		QStringLiteral("GuiFont! %1:h16:b:l").arg(GetPlatformTestFont())
+		QStringLiteral("GuiFont! DejaVu Sans Mono:h16:b:l")
 	};
-	const QString expectedFontBoldRemoved{ QStringLiteral("%1:h16:l").arg(GetPlatformTestFont()) };
-	QSignalSpy spy_fontchange2(w->shell(), &ShellWidget::shellFontChanged);
-	QSignalSpy cmd_gf2{ c->neovimObject()->vim_command_output(c->encode(cmdFontBoldRemoved)),
-						&MsgpackRequest::finished };
-	QVERIFY(cmd_gf2.isValid());
-	QVERIFY(SPYWAIT(cmd_gf2, 5000));
 
-	// Test Performance: timeout occurs often, set value carefully.
-	SPYWAIT(spy_fontchange2, 5000 /*msec*/);
+	const QString expectedFontBoldRemoved{ QStringLiteral("DejaVu Sans Mono:h16:l") };
+
+	QSignalSpy spyFontConflictingArgs{ c->neovimObject()->vim_command_output(
+										   c->encode(cmdFontBoldRemoved)),
+		&MsgpackRequest::finished };
+	QSignalSpy spyFontChangedBoldRemoved(w->shell(), &ShellWidget::shellFontChanged);
 
+	QVERIFY(spyFontConflictingArgs.isValid());
+	QVERIFY(SPYWAIT(spyFontConflictingArgs));
+	QVERIFY(SPYWAIT(spyFontChangedBoldRemoved));
 	QCOMPARE(w->shell()->fontDesc(), expectedFontBoldRemoved);
 }
 

test/tst_shell.h

@@ -8,8 +8,6 @@ namespace NeovimQt {
 
 QClipboard::Mode GetClipboardMode(char reg) noexcept;
 
-QString GetPlatformTestFont() noexcept;
-
 QStringList BinaryAndArgumentsNoForkWithCommand(const QString& command) noexcept;
 
 void AddPlatformSpecificExitCodeCases() noexcept;

test/tst_shell_mac.cpp

@@ -9,11 +9,6 @@ QClipboard::Mode GetClipboardMode(char /*reg*/) noexcept
 	return QClipboard::Clipboard;
 }
 
-QString GetPlatformTestFont() noexcept
-{
-	return QStringLiteral("Monaco");
-}
-
 QStringList BinaryAndArgumentsNoForkWithCommand(const QString& command) noexcept
 {
 	return { NVIM_QT_BINARY, "--nofork", "--",  "-c", command };

test/tst_shell_unix.cpp

@@ -13,11 +13,6 @@ QClipboard::Mode GetClipboardMode(char reg) noexcept
 	return QClipboard::Clipboard;
 }
 
-QString GetPlatformTestFont() noexcept
-{
-	return QStringLiteral("DejaVu Sans Mono");
-}
-
 QStringList BinaryAndArgumentsNoForkWithCommand(const QString& command) noexcept
 {
 	return { NVIM_QT_BINARY, "--nofork", "--",  "-c", command };

test/tst_shell_win32.cpp

@@ -9,11 +9,6 @@ QClipboard::Mode GetClipboardMode(char /*reg*/) noexcept
 	return QClipboard::Clipboard;
 }
 
-QString GetPlatformTestFont() noexcept
-{
-	return QStringLiteral("Consolas");
-}
-
 QStringList BinaryAndArgumentsNoForkWithCommand(const QString& command) noexcept
 {
 	return { NVIM_QT_BINARY, "--",  "-c", command };