diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-03 15:42:40 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-03 15:42:40 +0200 |
| commit | eb35acda4207392c10b4b1192b3b057bcad99a47 (patch) | |
| tree | c962e91fc502ec57d92df386e773033fe42fd17b /tests | |
| parent | afd20c9527fa77ba60901707c7bc73b2af926a67 (diff) | |
| parent | f62ced3c2c85675e746bff7ef8aca5c75c9737e0 (diff) | |
| download | qtmaildir-eb35acda4207392c10b4b1192b3b057bcad99a47.tar.gz qtmaildir-eb35acda4207392c10b4b1192b3b057bcad99a47.zip | |
Merge branch 'feature/qaction-menus'
Menus, a toolbar and a generated shortcut reference, built on converting
the action registry from a hash of callbacks to QActions. Along the way:
three default key bindings that had never fired, a shortcut dialog taller
than the screen, thread list columns that could not be resized, no visible
feedback that a tag action had landed, and a tags column so wide it was
unreadable.
Backlog items 3, 8, 9, 13 and 14 done; 11 partly.
Diffstat (limited to 'tests')
| -rw-r--r-- | tests/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | tests/test_keymap.cpp | 151 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 93 | ||||
| -rw-r--r-- | tests/test_tagcolors.cpp | 251 | ||||
| -rw-r--r-- | tests/test_threadlistmodel.cpp | 182 |
5 files changed, 659 insertions, 19 deletions
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 1833f29..e761cb6 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -13,6 +13,7 @@ target_compile_definitions(test_mimeparser PRIVATE add_qtmaildir_test(interceptor) add_qtmaildir_test(htmlbuilder) add_qtmaildir_test(notmuchworker) +add_qtmaildir_test(tagcolors) add_qtmaildir_test(threadlistmodel) add_qtmaildir_test(mailsync) add_qtmaildir_test(threadcidmap) diff --git a/tests/test_keymap.cpp b/tests/test_keymap.cpp index 7fc31ef..c81eeb0 100644 --- a/tests/test_keymap.cpp +++ b/tests/test_keymap.cpp @@ -32,18 +32,143 @@ private slots: void unknownActionIsReported(); void invalidSequenceIsReported(); void collidingOverridesAreReported(); + void bareCapitalMatchesShiftedPress(); + void userBindingWinsOverDefaultInMenus(); + void defaultsDoNotCollide(); + void everyDefaultIsAKnownAction(); }; void TestKeyMap::defaultsAreLoaded() { KeyMap map; map.loadDefaults(); - QCOMPARE(map.actionFor(QKeySequence(QStringLiteral("j"))), + QCOMPARE(map.actionFor(QKeySequence(QStringLiteral("Ctrl+J"))), QStringLiteral("next_thread")); - QCOMPARE(map.actionFor(QKeySequence(QStringLiteral("a"))), + QCOMPARE(map.actionFor(QKeySequence(QStringLiteral("Ctrl+E"))), QStringLiteral("archive")); } +void TestKeyMap::bareCapitalMatchesShiftedPress() +{ + // Typing a capital produces Shift+<key>, but QKeySequence::fromString() + // discards the case of a bare letter: "N" and "n" both parse to plain + // Key_N, which no keypress can ever produce. A user who writes "N = flag" + // would get a binding that silently never fires. Normalizing a bare + // capital to Shift+<key> is what they meant. + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("t.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("keys")); + s.setValue(QStringLiteral("N"), QStringLiteral("flag")); + s.endGroup(); + } + + KeyMap map; + map.loadDefaults(); + QSettings s(path, QSettings::IniFormat); + map.loadOverrides(s); + + // The sequence a real Shift+N keypress produces. + QKeyEvent press(QEvent::KeyPress, Qt::Key_N, Qt::ShiftModifier); + QCOMPARE(map.actionFor(QKeySequence(press.keyCombination())), + QStringLiteral("flag")); + + // A lowercase binding stays unshifted, so the two remain distinguishable. + QVERIFY(map.warnings().isEmpty()); + + // "y" and "Y" are two different keys, not a collision: the second would + // have silently displaced the first before normalization. + QTemporaryDir caseDir; + const QString casePath = caseDir.filePath(QStringLiteral("case.conf")); + { + QSettings s(casePath, QSettings::IniFormat); + s.beginGroup(QStringLiteral("keys")); + s.setValue(QStringLiteral("y"), QStringLiteral("archive")); + s.setValue(QStringLiteral("Y"), QStringLiteral("delete")); + s.endGroup(); + } + KeyMap caseMap; + QSettings caseSettings(casePath, QSettings::IniFormat); + caseMap.loadOverrides(caseSettings); + + QKeyEvent lower(QEvent::KeyPress, Qt::Key_Y, Qt::NoModifier); + QKeyEvent upper(QEvent::KeyPress, Qt::Key_Y, Qt::ShiftModifier); + QCOMPARE(caseMap.actionFor(QKeySequence(lower.keyCombination())), + QStringLiteral("archive")); + QCOMPARE(caseMap.actionFor(QKeySequence(upper.keyCombination())), + QStringLiteral("delete")); + QVERIFY(caseMap.warnings().isEmpty()); +} + +void TestKeyMap::userBindingWinsOverDefaultInMenus() +{ + // loadOverrides() adds a binding without removing the default, so two + // sequences reach 'archive'. sequenceFor() is what the menus and the + // shortcut reference display: it must show the user's, not the built-in + // one they were trying to replace. + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("t.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("keys")); + s.setValue(QStringLiteral("Ctrl+Alt+A"), QStringLiteral("archive")); + s.endGroup(); + } + + KeyMap map; + map.loadDefaults(); + QSettings s(path, QSettings::IniFormat); + map.loadOverrides(s); + + QCOMPARE(map.sequenceFor(QStringLiteral("archive")), + QKeySequence(QStringLiteral("Ctrl+Alt+A"))); + + // The default still fires; it is only no longer the advertised one. + QCOMPARE(map.actionFor(KeyMap::defaultSequenceFor(QStringLiteral("archive"))), + QStringLiteral("archive")); + + // An action the user left alone still shows its default. + QCOMPARE(map.sequenceFor(QStringLiteral("delete")), + KeyMap::defaultSequenceFor(QStringLiteral("delete"))); +} + +void TestKeyMap::defaultsDoNotCollide() +{ + // Two defaults on one sequence means one of them is unreachable, and the + // QHash would silently keep whichever was inserted last. + KeyMap map; + map.loadDefaults(); + + QSet<QString> actions; + for (const QString &action : KeyMap::knownActions()) { + const QKeySequence seq = map.defaultSequenceFor(action); + if (seq.isEmpty()) + continue; // Not every action carries a default. + QVERIFY2(map.actionFor(seq) == action, + qPrintable(QStringLiteral("default '%1' for '%2' resolves to '%3'") + .arg(seq.toString(), action, map.actionFor(seq)))); + actions.insert(action); + } + QVERIFY(!actions.isEmpty()); +} + +void TestKeyMap::everyDefaultIsAKnownAction() +{ + // A default bound to a name loadOverrides() would reject as unknown. + KeyMap map; + map.loadDefaults(); + const QStringList known = KeyMap::knownActions(); + for (const QString &action : known) + QVERIFY(!action.isEmpty()); + + for (const QString &action : map.defaultActions()) { + QVERIFY2(known.contains(action), + qPrintable(QStringLiteral("default binds unknown action '%1'") + .arg(action))); + } +} + void TestKeyMap::iniOverridesDefault() { QTemporaryDir dir; @@ -51,7 +176,7 @@ void TestKeyMap::iniOverridesDefault() { QSettings s(path, QSettings::IniFormat); s.beginGroup(QStringLiteral("keys")); - s.setValue(QStringLiteral("j"), QStringLiteral("archive")); + s.setValue(QStringLiteral("Ctrl+J"), QStringLiteral("archive")); s.endGroup(); } @@ -60,10 +185,10 @@ void TestKeyMap::iniOverridesDefault() QSettings s(path, QSettings::IniFormat); map.loadOverrides(s); - QCOMPARE(map.actionFor(QKeySequence(QStringLiteral("j"))), + QCOMPARE(map.actionFor(QKeySequence(QStringLiteral("Ctrl+J"))), QStringLiteral("archive")); // An untouched default survives. - QCOMPARE(map.actionFor(QKeySequence(QStringLiteral("k"))), + QCOMPARE(map.actionFor(QKeySequence(QStringLiteral("Ctrl+K"))), QStringLiteral("prev_thread")); } @@ -148,16 +273,22 @@ void TestKeyMap::invalidSequenceIsReported() void TestKeyMap::collidingOverridesAreReported() { - // "y" and "Y" both normalize to the same QKeySequence ("Y"), so binding - // both in [keys] is a genuine collision that must not silently drop one. + // Two spellings of one sequence. "Ctrl+Y" and "ctrl+y" parse identically, + // so binding both in [keys] is a genuine collision that must not silently + // drop one. + // + // Note "y" and "Y" are NOT a collision any more: normalizeSequence() + // rewrites a bare capital to Shift+Y, which is the key a user actually + // presses, leaving the two distinct. Before that they both folded to + // plain Key_Y and one was lost. { QTemporaryDir dir; const QString path = dir.filePath(QStringLiteral("t.conf")); { QSettings s(path, QSettings::IniFormat); s.beginGroup(QStringLiteral("keys")); - s.setValue(QStringLiteral("y"), QStringLiteral("archive")); - s.setValue(QStringLiteral("Y"), QStringLiteral("delete")); + s.setValue(QStringLiteral("Ctrl+Y"), QStringLiteral("archive")); + s.setValue(QStringLiteral("ctrl+y"), QStringLiteral("delete")); s.endGroup(); } @@ -179,7 +310,7 @@ void TestKeyMap::collidingOverridesAreReported() { QSettings s(path, QSettings::IniFormat); s.beginGroup(QStringLiteral("keys")); - s.setValue(QStringLiteral("j"), QStringLiteral("archive")); + s.setValue(QStringLiteral("Ctrl+J"), QStringLiteral("archive")); s.endGroup(); } diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 57eb763..6bfa925 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -18,18 +18,27 @@ #include <QtTest> +#include <QAction> +#include <QDir> +#include <QSettings> +#include <QTemporaryDir> + +#include "config.h" #include "keymap.h" #include "mainwindow.h" -/// MainWindow is mostly wiring and needs a live QApplication plus a real -/// database, so it is verified manually in Task 13. Two things do not need -/// either, and both are the kind of drift a comment alone does not prevent. +/// MainWindow is mostly wiring, and the parts that need a real database are +/// still verified manually. What is checked here is the action registry: the +/// bindings a user configures reach the QActions the menus and the keyboard +/// both read from, and no action is left unreachable. class TestMainWindow : public QObject { Q_OBJECT private slots: void everyKnownActionIsRegistered(); void everyRegisteredActionIsKnown(); + void everyActionHasAShortcut(); + void configuredBindingReachesTheAction(); void cidPrefixesAreBangFree(); void cidPrefixesAreDistinctPerMessage(); }; @@ -39,8 +48,14 @@ void TestMainWindow::everyKnownActionIsRegistered() // KeyMap::knownActions() is what loadOverrides() validates config bindings // against. An action listed there but never registered means a user can // bind a key in qtmaildir.conf, get no warning, and have it do nothing. + // + // registeredActionNames() is now derived from the QActions themselves, so + // this compares against what the window really installed. + const Config config; + MainWindow window(config); + const QStringList known = KeyMap::knownActions(); - const QStringList registered = MainWindow::registeredActionNames(); + const QStringList registered = window.registeredActionNames(); for (const QString &action : known) { QVERIFY2(registered.contains(action), @@ -53,8 +68,11 @@ void TestMainWindow::everyRegisteredActionIsKnown() { // The reverse drift: an action MainWindow implements but KeyMap rejects. // The user would get "unknown action" for a binding that is really there. + const Config config; + MainWindow window(config); + const QStringList known = KeyMap::knownActions(); - const QStringList registered = MainWindow::registeredActionNames(); + const QStringList registered = window.registeredActionNames(); for (const QString &action : registered) { QVERIFY2(known.contains(action), @@ -63,6 +81,57 @@ void TestMainWindow::everyRegisteredActionIsKnown() } } +void TestMainWindow::everyActionHasAShortcut() +{ + // An action with no binding is unreachable from the keyboard. Every one + // of them carries a default, so an empty shortcut means the default table + // and the action list have drifted apart. + const Config config; + MainWindow window(config); + + for (const QString &name : window.registeredActionNames()) { + const QAction *action = window.findChild<QAction *>(name); + QVERIFY2(action, qPrintable(QStringLiteral("no QAction named '%1'").arg(name))); + QVERIFY2(!action->shortcut().isEmpty(), + qPrintable(QStringLiteral("action '%1' has no shortcut").arg(name))); + } +} + +void TestMainWindow::configuredBindingReachesTheAction() +{ + // The whole point of [keys]: a user's override must end up on the QAction, + // which is what both the keyboard and the menus read. + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("qtmaildir.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("keys")); + s.setValue(QStringLiteral("Ctrl+Alt+A"), QStringLiteral("archive")); + s.endGroup(); + } + + // MainWindow reads its keymap from Config::defaultPath(), so point that + // at the temporary file for this test. + const QString previous = qEnvironmentVariable("XDG_CONFIG_HOME"); + QVERIFY(QDir().mkpath(dir.filePath(QStringLiteral("qtmaildir")))); + QVERIFY(QFile::copy(path, dir.filePath(QStringLiteral("qtmaildir/qtmaildir.conf")))); + qputenv("XDG_CONFIG_HOME", dir.path().toUtf8()); + + { + const Config config; + MainWindow window(config); + const QAction *archive = + window.findChild<QAction *>(QStringLiteral("archive")); + QVERIFY(archive); + QCOMPARE(archive->shortcut(), QKeySequence(QStringLiteral("Ctrl+Alt+A"))); + } + + if (previous.isEmpty()) + qunsetenv("XDG_CONFIG_HOME"); + else + qputenv("XDG_CONFIG_HOME", previous.toUtf8()); +} + void TestMainWindow::cidPrefixesAreBangFree() { // MainWindow is the only producer of cidPrefix in the application. The @@ -89,5 +158,17 @@ void TestMainWindow::cidPrefixesAreDistinctPerMessage() } } -QTEST_MAIN(TestMainWindow) +// Constructing a MainWindow needs a QApplication and a platform plugin. The +// test has no display under ctest, so it runs offscreen unless the caller +// asked for something else. +int main(int argc, char *argv[]) +{ + qputenv("QT_QPA_PLATFORM", qgetenv("QT_QPA_PLATFORM").isEmpty() + ? QByteArray("offscreen") + : qgetenv("QT_QPA_PLATFORM")); + QApplication app(argc, argv); + TestMainWindow test; + return QTest::qExec(&test, argc, argv); +} + #include "test_mainwindow.moc" diff --git a/tests/test_tagcolors.cpp b/tests/test_tagcolors.cpp new file mode 100644 index 0000000..53c0210 --- /dev/null +++ b/tests/test_tagcolors.cpp @@ -0,0 +1,251 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * 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, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include <QSettings> +#include <QTemporaryDir> +#include <QtTest> + +#include "tagcolors.h" + +class TestTagColors : public QObject +{ + Q_OBJECT +private slots: + void builtInDefaultsExist(); + void prefixColoursWholeHierarchy(); + void exactTagBeatsItsPrefix(); + void configOverridesABuiltIn(); + void unknownTagStillGetsAColour(); + void accountTagsAreRecognised(); + void accountColourComesFromTheAccount(); + void accountLabelDefaultsToTheKey(); + void accountLabelCanBeOverridden(); + void malformedColourIsReported(); + void textContrastsWithItsBackground(); +}; + +void TestTagColors::builtInDefaultsExist() +{ + // The common state tags must be styled out of the box: a user who never + // writes a [tagcolors] section still needs flagged to stand out. + TagColors colours; + const QStringList expected = { QStringLiteral("flagged"), + QStringLiteral("unread"), + QStringLiteral("deleted"), + QStringLiteral("spam"), + QStringLiteral("attachment"), + QStringLiteral("replied") }; + for (const QString &tag : expected) { + QVERIFY2(colours.hasColour(tag), + qPrintable(QStringLiteral("no built-in colour for '%1'").arg(tag))); + } +} + +void TestTagColors::prefixColoursWholeHierarchy() +{ + // 96 tags, many of them shopping/foo and mailing-list/bar. Colouring by + // top-level prefix is what keeps the config from listing every one. + TagColors colours; + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("t.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("tagcolors")); + s.setValue(QStringLiteral("shopping"), QStringLiteral("#3366cc")); + s.endGroup(); + } + QSettings s(path, QSettings::IniFormat); + colours.load(s); + + QCOMPARE(colours.colourFor(QStringLiteral("shopping/amazon")), + QColor(QStringLiteral("#3366cc"))); + QCOMPARE(colours.colourFor(QStringLiteral("shopping/nike")), + QColor(QStringLiteral("#3366cc"))); + // The bare prefix itself is a tag too. + QCOMPARE(colours.colourFor(QStringLiteral("shopping")), + QColor(QStringLiteral("#3366cc"))); + // A different hierarchy is unaffected. + QVERIFY(colours.colourFor(QStringLiteral("mailing-list/SBo")) + != QColor(QStringLiteral("#3366cc"))); +} + +void TestTagColors::exactTagBeatsItsPrefix() +{ + // Specific beats general, or you could never single out one child tag. + TagColors colours; + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("t.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("tagcolors")); + s.setValue(QStringLiteral("shopping"), QStringLiteral("#3366cc")); + s.setValue(QStringLiteral("shopping/amazon"), QStringLiteral("#ff9900")); + s.endGroup(); + } + QSettings s(path, QSettings::IniFormat); + colours.load(s); + + QCOMPARE(colours.colourFor(QStringLiteral("shopping/amazon")), + QColor(QStringLiteral("#ff9900"))); + QCOMPARE(colours.colourFor(QStringLiteral("shopping/nike")), + QColor(QStringLiteral("#3366cc"))); + + // Regression: QSettings treats '/' as a group separator, so a + // hierarchical tag is a nested key that childKeys() never returns. Reading + // the group with childKeys() silently dropped every tag with a '/' in it, + // which is most of this user's, and they all fell through to their prefix. + QVERIFY(colours.hasColour(QStringLiteral("shopping/amazon"))); +} + +void TestTagColors::configOverridesABuiltIn() +{ + TagColors colours; + const QColor original = colours.colourFor(QStringLiteral("flagged")); + + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("t.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("tagcolors")); + s.setValue(QStringLiteral("flagged"), QStringLiteral("#00ff00")); + s.endGroup(); + } + QSettings s(path, QSettings::IniFormat); + colours.load(s); + + QCOMPARE(colours.colourFor(QStringLiteral("flagged")), + QColor(QStringLiteral("#00ff00"))); + QVERIFY(colours.colourFor(QStringLiteral("flagged")) != original); +} + +void TestTagColors::unknownTagStillGetsAColour() +{ + // A chip with no colour would render as an invisible blank, so every tag + // resolves to something even when nothing is configured for it. + TagColors colours; + const QColor colour = colours.colourFor(QStringLiteral("no-such-tag-anywhere")); + QVERIFY(colour.isValid()); + + // Stable across calls: a tag must not change colour as you scroll. + QCOMPARE(colours.colourFor(QStringLiteral("no-such-tag-anywhere")), colour); +} + +void TestTagColors::accountTagsAreRecognised() +{ + // Account tags are a different taxonomy from functional tags: which + // mailbox a thread came from, not what state it is in. They are shown + // separately, so they have to be identifiable. + QVERIFY(TagColors::isAccountTag(QStringLiteral("account-gmail-danixland"))); + QVERIFY(!TagColors::isAccountTag(QStringLiteral("flagged"))); + QVERIFY(!TagColors::isAccountTag(QStringLiteral("shopping/amazon"))); + + // The INI key for [account.gmail-danixland] is what follows "account-". + QCOMPARE(TagColors::accountKeyForTag(QStringLiteral("account-gmail-danixland")), + QStringLiteral("gmail-danixland")); + QVERIFY(TagColors::accountKeyForTag(QStringLiteral("flagged")).isEmpty()); + + // Round trip, since the mapping is derived rather than configured. + QCOMPARE(TagColors::tagForAccountKey(QStringLiteral("gmail-danixland")), + QStringLiteral("account-gmail-danixland")); +} + +void TestTagColors::accountColourComesFromTheAccount() +{ + // Per the account stanza, not [tagcolors]: the colour belongs to the + // account, and the tag name is derived from its key. + TagColors colours; + colours.setAccountColour(QStringLiteral("gmail-danixland"), + QColor(QStringLiteral("#cc0000"))); + + QCOMPARE(colours.colourFor(QStringLiteral("account-gmail-danixland")), + QColor(QStringLiteral("#cc0000"))); +} + +void TestTagColors::accountLabelDefaultsToTheKey() +{ + // Without a configured label the chip shows the account key, which is what + // it did before labels existed. + TagColors colours; + QCOMPARE(colours.labelForAccountTag(QStringLiteral("account-gmail-danixland")), + QStringLiteral("gmail-danixland")); + + // Not an account tag: nothing to label. + QVERIFY(colours.labelForAccountTag(QStringLiteral("flagged")).isEmpty()); +} + +void TestTagColors::accountLabelCanBeOverridden() +{ + // "account-privateemail-danilo.macri" is 33 characters of chip for what is + // really one bit of information, so the label is configurable. + TagColors colours; + colours.setAccountLabel(QStringLiteral("gmail-danixland"), + QStringLiteral("GM-danixland")); + colours.setAccountLabel(QStringLiteral("privateemail-danix"), + QStringLiteral("PE-danix")); + + QCOMPARE(colours.labelForAccountTag(QStringLiteral("account-gmail-danixland")), + QStringLiteral("GM-danixland")); + QCOMPARE(colours.labelForAccountTag(QStringLiteral("account-privateemail-danix")), + QStringLiteral("PE-danix")); + + // An account left unlabelled still falls back to its key. + QCOMPARE(colours.labelForAccountTag(QStringLiteral("account-work")), + QStringLiteral("work")); + + // An empty label is not an override: it would render a blank chip. + colours.setAccountLabel(QStringLiteral("gmail-danixland"), QString()); + QCOMPARE(colours.labelForAccountTag(QStringLiteral("account-gmail-danixland")), + QStringLiteral("GM-danixland")); +} + +void TestTagColors::malformedColourIsReported() +{ + // A typo must be visible rather than silently ignored, matching how the + // rest of the config reports its problems. + TagColors colours; + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("t.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("tagcolors")); + s.setValue(QStringLiteral("flagged"), QStringLiteral("not-a-colour")); + s.endGroup(); + } + QSettings s(path, QSettings::IniFormat); + colours.load(s); + + QCOMPARE(colours.warnings().size(), 1); + QVERIFY(colours.warnings().first().contains(QStringLiteral("flagged"))); + // The built-in survives, so one bad line does not leave the tag unstyled. + QVERIFY(colours.colourFor(QStringLiteral("flagged")).isValid()); +} + +void TestTagColors::textContrastsWithItsBackground() +{ + // A chip is coloured text on a coloured fill, so the pair has to stay + // legible whatever colour the user picks. + QCOMPARE(TagColors::textColourOn(QColor(Qt::black)), QColor(Qt::white)); + QCOMPARE(TagColors::textColourOn(QColor(Qt::white)), QColor(Qt::black)); + QCOMPARE(TagColors::textColourOn(QColor(QStringLiteral("#8b2c2c"))), + QColor(Qt::white)); + QCOMPARE(TagColors::textColourOn(QColor(QStringLiteral("#ffee88"))), + QColor(Qt::black)); +} + +QTEST_MAIN(TestTagColors) +#include "test_tagcolors.moc" diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp index 24ba9e2..e8a5fa8 100644 --- a/tests/test_threadlistmodel.cpp +++ b/tests/test_threadlistmodel.cpp @@ -33,6 +33,14 @@ private slots: void reportsSubjectAndAuthors(); void subjectShowsMessageCountOnlyForRealThreads(); void unreadThreadsRenderBold(); + void tagsAreTheFirstColumnAndSubjectTheLast(); + void accountTagBecomesAChipLabel(); + void unreadStylingSurvivesAnAccountChip(); + void accountChipUsesTheConfiguredColour(); + void deletedThreadsAreRedAndStruckThrough(); + void spamThreadsAreOrangeAndStruckThrough(); + void doomedStylingCoversEveryColumn(); + void ordinaryThreadsCarryNoRowColour(); void threadIdIsReachableFromAnIndex(); void invalidIndexesReturnNothing(); void threadAtOutOfRangeIsSafe(); @@ -113,9 +121,11 @@ void TestThreadListModel::reportsSubjectAndAuthors() const QModelIndex date = model.index(0, ThreadListModel::DateColumn); QVERIFY(!model.data(date, Qt::DisplayRole).toString().isEmpty()); - const QModelIndex tags = model.index(0, ThreadListModel::TagsColumn); - QCOMPARE(model.data(tags, Qt::DisplayRole).toString(), - QStringLiteral("inbox unread")); + // Tags are no longer a column; they reach the strip under the message + // pane through a role instead. + const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + QCOMPARE(model.data(subject, ThreadListModel::TagsRole).toStringList(), + QStringList({ QStringLiteral("inbox"), QStringLiteral("unread") })); } void TestThreadListModel::subjectShowsMessageCountOnlyForRealThreads() @@ -153,6 +163,172 @@ void TestThreadListModel::unreadThreadsRenderBold() QVERIFY(unreadFont.value<QFont>().bold()); } +void TestThreadListModel::tagsAreTheFirstColumnAndSubjectTheLast() +{ + // Subject stretches to fill the view, so whatever sits after it is pushed + // off-screen. Tags used to be there, which is why acting on a thread + // looked like it did nothing: the only column that changed was invisible. + QCOMPARE(ThreadListModel::SubjectColumn, ThreadListModel::ColumnCount - 1); + + ThreadListModel model; + model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("hello")) }); + QCOMPARE(model.headerData(ThreadListModel::SubjectColumn, Qt::Horizontal, + Qt::DisplayRole).toString(), + QStringLiteral("Subject")); + + // No tags column at all: spelling out a dozen tags per row consumed most + // of the list's width and was unreadable. + for (int column = 0; column < ThreadListModel::ColumnCount; ++column) { + QVERIFY(model.headerData(column, Qt::Horizontal, Qt::DisplayRole) + .toString() != QStringLiteral("Tags")); + } +} + +void TestThreadListModel::accountTagBecomesAChipLabel() +{ + // The account tag is a different taxonomy from a functional one: which + // mailbox the thread arrived in. It renders as a chip in front of the + // subject, so the model exposes its label and colour separately. + ThreadListModel model; + ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("hello")); + thread.tags = QStringList{ QStringLiteral("inbox"), + QStringLiteral("account-gmail-danixland") }; + model.appendBatch({ thread }); + + const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + QCOMPARE(model.data(subject, ThreadListModel::AccountLabelRole).toString(), + QStringLiteral("gmail-danixland")); + QVERIFY(model.data(subject, ThreadListModel::AccountColourRole) + .value<QColor>().isValid()); + + // A thread with no account tag gets no chip rather than an empty one. + ThreadListModel plain; + ThreadSummary untagged = makeThread(QStringLiteral("t2"), QStringLiteral("hi")); + untagged.tags = QStringList{ QStringLiteral("inbox") }; + plain.appendBatch({ untagged }); + QVERIFY(plain.data(plain.index(0, ThreadListModel::SubjectColumn), + ThreadListModel::AccountLabelRole).toString().isEmpty()); +} + +void TestThreadListModel::unreadStylingSurvivesAnAccountChip() +{ + // The subject cell is drawn by a delegate when the thread has an account + // chip. The delegate paints the text itself, so it has to keep honouring + // the model's font: otherwise an unread thread stops rendering bold for + // exactly those threads that carry an account tag, which is all of them. + ThreadListModel model; + ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("hello")); + thread.tags = QStringList{ QStringLiteral("inbox"), QStringLiteral("unread"), + QStringLiteral("account-gmail-danixland") }; + model.appendBatch({ thread }); + + const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + QVERIFY(!model.data(subject, ThreadListModel::AccountLabelRole) + .toString().isEmpty()); + + const QVariant font = model.data(subject, Qt::FontRole); + QVERIFY2(font.isValid(), "unread thread with an account tag has no font"); + QVERIFY2(font.value<QFont>().bold(), "unread thread is not bold"); +} + +void TestThreadListModel::accountChipUsesTheConfiguredColour() +{ + // The colour comes from the account's own stanza, so a configured one must + // reach the chip rather than the generated fallback. + TagColors colours; + colours.setAccountColour(QStringLiteral("gmail-danixland"), + QColor(QStringLiteral("#cc0000"))); + + ThreadListModel model; + model.setTagColors(&colours); + ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("hello")); + thread.tags = QStringList{ QStringLiteral("account-gmail-danixland") }; + model.appendBatch({ thread }); + + QCOMPARE(model.data(model.index(0, ThreadListModel::SubjectColumn), + ThreadListModel::AccountColourRole).value<QColor>(), + QColor(QStringLiteral("#cc0000"))); +} + +void TestThreadListModel::deletedThreadsAreRedAndStruckThrough() +{ + ThreadListModel model; + ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("doomed")); + thread.tags = QStringList{ QStringLiteral("inbox") }; + model.appendBatch({ thread }); + + const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + QVERIFY(!model.data(subject, Qt::BackgroundRole).isValid()); + + model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {}); + + const QVariant background = model.data(subject, Qt::BackgroundRole); + QVERIFY(background.isValid()); + QCOMPARE(background.value<QBrush>().color(), ThreadListModel::deletedColour()); + + // White text on the fill, and struck through so the state reads even in a + // screenshot with the colours stripped. + QCOMPARE(model.data(subject, Qt::ForegroundRole).value<QBrush>().color(), + QColor(Qt::white)); + QVERIFY(model.data(subject, Qt::FontRole).value<QFont>().strikeOut()); +} + +void TestThreadListModel::spamThreadsAreOrangeAndStruckThrough() +{ + ThreadListModel model; + ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("junk")); + thread.tags = QStringList{ QStringLiteral("inbox") }; + model.appendBatch({ thread }); + + model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("spam") }, {}); + + const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + QCOMPARE(model.data(subject, Qt::BackgroundRole).value<QBrush>().color(), + ThreadListModel::spamColour()); + QVERIFY(model.data(subject, Qt::FontRole).value<QFont>().strikeOut()); + + // Spam and deleted must be distinguishable, not two shades of one colour. + QVERIFY(ThreadListModel::spamColour() != ThreadListModel::deletedColour()); +} + +void TestThreadListModel::doomedStylingCoversEveryColumn() +{ + // A cue on one column would vanish the moment that column scrolled out of + // view, which is the bug this whole change exists to fix. + ThreadListModel model; + ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("doomed")); + thread.tags = QStringList{ QStringLiteral("inbox") }; + model.appendBatch({ thread }); + + model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {}); + + for (int column = 0; column < ThreadListModel::ColumnCount; ++column) { + const QModelIndex index = model.index(0, column); + QVERIFY2(model.data(index, Qt::BackgroundRole).isValid(), + qPrintable(QStringLiteral("column %1 has no background").arg(column))); + QVERIFY2(model.data(index, Qt::FontRole).value<QFont>().strikeOut(), + qPrintable(QStringLiteral("column %1 is not struck through").arg(column))); + } +} + +void TestThreadListModel::ordinaryThreadsCarryNoRowColour() +{ + // Undo has to restore the plain look, not merely drop the tag. + ThreadListModel model; + ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("normal")); + thread.tags = QStringList{ QStringLiteral("inbox") }; + model.appendBatch({ thread }); + + model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {}); + model.applyTagChange(QStringLiteral("t1"), {}, { QStringLiteral("deleted") }); + + const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + QVERIFY(!model.data(subject, Qt::BackgroundRole).isValid()); + QVERIFY(!model.data(subject, Qt::ForegroundRole).isValid()); + const QVariant font = model.data(subject, Qt::FontRole); + QVERIFY(!font.isValid() || !font.value<QFont>().strikeOut()); +} + void TestThreadListModel::threadIdIsReachableFromAnIndex() { // The view hands MainWindow a QModelIndex; the worker needs a thread id. |
