From e22a75974a49b64beddf7189e35914c812facade Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Wed, 4 Nov 2026 18:39:42 +0100 Subject: [PATCH] [Icons] Add an icon asset ratchet test (#7596) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Icons] Add an icon asset ratchet test The icon rules were enforced by review only, so the next Inkscape export could reintroduce a baked fill or a physical size without anyone noticing. Add a test that rejects non-24 viewBoxes, physical sizes, editor metadata, hardcoded paints, missing tokens, color-coded filenames, and rasters. Keep the legacy baseline empty so the ratchet can only tighten. * [Tests] Drop the empty icon baseline The legacy icon set is fully paid down, so the ratchet had nothing left to ratchet: BaselineIsWellFormed required the baseline to be empty, which made the baseline file, the stale-entry diff and the entry-format regex unreachable. The rules are enforced directly now. - AssetsFollowTheRules fails on any violation of collectViolations(). - tests/icon_assets/icon-baseline.txt is gone with its reader. --------- Co-authored-by: Lukas Brübach --- tests/CMakeLists.txt | 1 + tests/icon_assets/CMakeLists.txt | 3 + tests/icon_assets/icon_assets_test.cpp | 241 +++++++++++++++++++++++++ 3 files changed, 245 insertions(+) create mode 100644 tests/icon_assets/CMakeLists.txt create mode 100644 tests/icon_assets/icon_assets_test.cpp diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 8b904d30a..84b9f02bf 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -566,6 +566,7 @@ add_subdirectory(card_zone_algorithms) add_subdirectory(carddatabase) add_subdirectory(deck_list_model) add_subdirectory(deck_list_zones) +add_subdirectory(icon_assets) add_subdirectory(loading_from_clipboard) add_subdirectory(model_contract) add_subdirectory(movecard_tests) diff --git a/tests/icon_assets/CMakeLists.txt b/tests/icon_assets/CMakeLists.txt new file mode 100644 index 000000000..da9776f8e --- /dev/null +++ b/tests/icon_assets/CMakeLists.txt @@ -0,0 +1,3 @@ +cockatrice_add_test( + NAME icon_assets_test SOURCES icon_assets_test.cpp DEFINITIONS COCKATRICE_SOURCE_DIR="${CMAKE_SOURCE_DIR}" +) diff --git a/tests/icon_assets/icon_assets_test.cpp b/tests/icon_assets/icon_assets_test.cpp new file mode 100644 index 000000000..f0b06d5cc --- /dev/null +++ b/tests/icon_assets/icon_assets_test.cpp @@ -0,0 +1,241 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace +{ +const QString ICONS_RELATIVE_PATH = QStringLiteral("cockatrice/resources/icons"); + +QString iconsDir() +{ + return QDir(QStringLiteral(COCKATRICE_SOURCE_DIR)).absoluteFilePath(ICONS_RELATIVE_PATH); +} + +/** @brief True for content art that is allowed to carry full color. */ +bool isContentArt(const QString &relativePath) +{ + return relativePath.startsWith(QStringLiteral("mana/")); +} + +QString violation(const QString &rule, const QString &relativePath) +{ + return rule + QLatin1Char(' ') + relativePath; +} + +/** + * @brief Reads one SVG and returns true when the root carries a usable viewBox. + * + * Also reports malformed XML and editor-only metadata through the out params. + */ +bool inspectSvg(const QString &absolutePath, + bool &validXml, + bool &hasViewBox, + bool &hasEditorMetadata, + bool &hasPhysicalSize) +{ + validXml = false; + hasViewBox = false; + hasEditorMetadata = false; + hasPhysicalSize = false; + + QFile file(absolutePath); + if (!file.open(QIODevice::ReadOnly)) { + return false; + } + + QXmlStreamReader reader(&file); + bool sawRoot = false; + + while (!reader.atEnd()) { + reader.readNext(); + + if (reader.hasError()) { + return false; + } + + if (!reader.isStartElement()) { + continue; + } + + if (!sawRoot) { + sawRoot = true; + if (reader.name() != QStringLiteral("svg")) { + return false; + } + const QString viewBox = reader.attributes().value(QStringLiteral("viewBox")).toString().simplified(); + hasViewBox = viewBox == QStringLiteral("0 0 24 24"); + hasPhysicalSize = !reader.attributes().value(QStringLiteral("width")).isEmpty() || + !reader.attributes().value(QStringLiteral("height")).isEmpty(); + } + + if (reader.name() == QStringLiteral("metadata") || reader.name() == QStringLiteral("namedview")) { + hasEditorMetadata = true; + } + + for (const QXmlStreamAttribute &attribute : reader.attributes()) { + const QString prefix = attribute.prefix().toString(); + if (prefix == QStringLiteral("inkscape") || prefix == QStringLiteral("sodipodi") || + prefix == QStringLiteral("dc") || prefix == QStringLiteral("cc") || prefix == QStringLiteral("rdf")) { + hasEditorMetadata = true; + } + } + } + + validXml = sawRoot && !reader.hasError(); + return validXml; +} + +bool isAllowedPaintValue(const QString &value) +{ + const QString normalized = value.trimmed().toLower(); + return normalized.isEmpty() || normalized == QStringLiteral("none") || + normalized == QStringLiteral("currentcolor") || normalized == QStringLiteral("inherit") || + normalized == QStringLiteral("context-fill") || normalized == QStringLiteral("context-stroke") || + normalized.startsWith(QStringLiteral("url(")); +} + +QRegularExpression paintValuePattern() +{ + static const QRegularExpression pattern( + QStringLiteral(R"re((?:fill|stroke|stop-color|color)\s*[:=]\s*["']?\s*([^"'\s;>]+))re"), + QRegularExpression::CaseInsensitiveOption); + return pattern; +} + +bool hasHardcodedColor(const QString &absolutePath) +{ + QFile file(absolutePath); + if (!file.open(QIODevice::ReadOnly)) { + return false; + } + + const QString content = QString::fromUtf8(file.readAll()); + auto matches = paintValuePattern().globalMatch(content); + while (matches.hasNext()) { + const QString value = matches.next().captured(1).trimmed(); + if (isAllowedPaintValue(value)) { + continue; + } + const QString normalized = value.toLower(); + if (normalized.startsWith(QLatin1Char('#')) || normalized.contains(QStringLiteral("rgb(")) || + normalized.contains(QStringLiteral("hsl(")) || QColor(value).isValid()) { + return true; + } + } + return false; +} + +bool hasCurrentColorToken(const QString &absolutePath) +{ + QFile file(absolutePath); + if (!file.open(QIODevice::ReadOnly)) { + return false; + } + + const QString content = QString::fromUtf8(file.readAll()); + auto matches = paintValuePattern().globalMatch(content); + while (matches.hasNext()) { + if (matches.next().captured(1).trimmed().compare(QStringLiteral("currentColor"), Qt::CaseInsensitive) == 0) { + return true; + } + } + return false; +} + +bool isSchemeVariant(const QString &fileName) +{ + return fileName.contains(QStringLiteral("-dark."), Qt::CaseInsensitive) || + fileName.contains(QStringLiteral("-light."), Qt::CaseInsensitive); +} + +bool hasColorInFilename(const QString &fileName) +{ + static const QRegularExpression colorSuffix( + QStringLiteral(R"(_(green|blue|red|yellow|black|white|gray|grey)\.svg$)"), + QRegularExpression::CaseInsensitiveOption); + return colorSuffix.match(fileName).hasMatch(); +} + +/** @brief Collects every current violation as "rule relative/path.svg". */ +QSet collectViolations() +{ + QSet violations; + + QDirIterator it(iconsDir(), QDir::Files, QDirIterator::Subdirectories); + while (it.hasNext()) { + it.next(); + const QFileInfo info = it.fileInfo(); + const QString relativePath = QDir(iconsDir()).relativeFilePath(info.absoluteFilePath()); + if (isContentArt(relativePath)) { + continue; + } + if (info.suffix().compare(QStringLiteral("svg"), Qt::CaseInsensitive) != 0) { + violations.insert(violation(QStringLiteral("raster"), relativePath)); + continue; + } + + bool validXml = false; + bool hasViewBox = false; + bool hasEditorMetadata = false; + bool hasPhysicalSize = false; + inspectSvg(info.absoluteFilePath(), validXml, hasViewBox, hasEditorMetadata, hasPhysicalSize); + + if (!validXml) { + violations.insert(violation(QStringLiteral("xml"), relativePath)); + } + if (validXml && !hasViewBox) { + violations.insert(violation(QStringLiteral("viewbox"), relativePath)); + } + if (hasPhysicalSize) { + violations.insert(violation(QStringLiteral("physical-size"), relativePath)); + } + if (hasEditorMetadata) { + violations.insert(violation(QStringLiteral("editor-metadata"), relativePath)); + } + if (hasHardcodedColor(info.absoluteFilePath()) && !isSchemeVariant(info.fileName())) { + violations.insert(violation(QStringLiteral("hardcoded-color"), relativePath)); + } + if (!hasCurrentColorToken(info.absoluteFilePath()) && !isSchemeVariant(info.fileName())) { + violations.insert(violation(QStringLiteral("no-token"), relativePath)); + } + if (hasColorInFilename(info.fileName())) { + violations.insert(violation(QStringLiteral("color-filename"), relativePath)); + } + } + + return violations; +} + +QString formatSet(const QSet &entries) +{ + QStringList lines(entries.begin(), entries.end()); + lines.sort(); + return lines.join(QLatin1Char('\n')); +} + +} // namespace + +/** + * @brief Every UI glyph follows the palette-token rules. + * + * The legacy set was paid down icon by icon, so no baseline grandfathers + * violations: a new violation fails right here. Scheme variants + * (`*-dark.svg`/`*-light.svg`) and mana art are exempt inside collectViolations(). + */ +TEST(IconAssets, AssetsFollowTheRules) +{ + ASSERT_TRUE(QDir::isAbsolutePath(iconsDir())); + ASSERT_TRUE(QDir(iconsDir()).exists()) << "Missing icon directory: " << iconsDir().toStdString(); + + const QSet violations = collectViolations(); + EXPECT_TRUE(violations.isEmpty()) << "Icon rule violations. Fix the icon; do not add exceptions:\n" + << formatSet(violations).toStdString(); +}