test: require meaningful corpus and embedded-font checks

scripts/tests/test_font_checks.py

@@ -0,0 +1,32 @@
+import base64
+import sys
+import tempfile
+import unittest
+from pathlib import Path
+from unittest.mock import patch
+
+sys.path.insert(0, str(Path(__file__).resolve().parents[2] / "test/scripts"))
+import check_fonts
+
+
+class FontChecksTest(unittest.TestCase):
+    def test_each_embedding_keeps_its_family_without_duplicate_orphans(self):
+        payload = base64.b64encode(b"font bytes").decode()
+        url = f"data:font/ttf;base64,{payload}"
+        html = (f"<style>@font-face{{font-family:'A';src:url({url});}}"
+                f"@font-face{{font-family:'B';src:url({url});}}</style>"
+                f"<p>{url}</p>")
+        with tempfile.TemporaryDirectory() as directory:
+            path = Path(directory) / "fonts.html"
+            path.write_text(html)
+            self.assertEqual(list(check_fonts.fonts_in(path)), [
+                ("A", b"font bytes"), ("B", b"font bytes"),
+                ("(no @font-face)", b"font bytes"),
+            ])
+
+    def test_empty_output_is_not_a_successful_check(self):
+        with tempfile.TemporaryDirectory() as directory:
+            with patch.object(sys, "argv", ["check_fonts.py", directory]), \
+                 patch.object(check_fonts, "find_ots", return_value=[]):
+                with self.assertRaisesRegex(SystemExit, "No HTML files"):
+                    check_fonts.main()

test/scripts/check_fonts.py

@@ -59,15 +59,15 @@ def html_files(roots):
 
 
 def fonts_in(path):
-    """(family, bytes) for every font the file embeds, in document order."""
+    """Embedded fonts with their declared family, if any."""
     with open(path, encoding="utf-8", errors="replace") as file:
         content = file.read()
     seen_at = set()
     for face in FONT_FACE.finditer(content):
         body = face.group(1)
         family = match.group(1) if (match := FONT_FAMILY.search(body)) else "?"
         for data in FONT_DATA_URL.finditer(body):
-            seen_at.add(face.start() + data.start())
+            seen_at.add(face.start(1) + data.start())
             yield family, decode(data.group(1), path, family)
     for data in FONT_DATA_URL.finditer(content):
         if data.start() not in seen_at:
@@ -110,13 +110,18 @@ def main():
     # distinct payload once and report every place it came from.
     verdicts = {}
     occurrences = {}
+    files = 0
     for path in html_files(arguments.roots):
+        files += 1
         for family, font in fonts_in(path):
             digest = hashlib.sha256(font).hexdigest()
             if digest not in verdicts:
                 verdicts[digest] = sanitize(ots, font)
             occurrences.setdefault(digest, []).append((path, family))
 
+    if files == 0:
+        sys.exit("No HTML files found to check")
+
     failed = {d: v for d, v in verdicts.items() if v is not None}
     for digest, complaint in sorted(failed.items()):
         where = occurrences[digest]

test/src/formula_corpus_test.cpp

@@ -125,9 +125,8 @@ void evaluate_file(const TestFile &file, Tally &tally) {
 
 } // namespace
 
-/// Decision 29 of `docs/design/spreadsheet-editing.md`: an answer is never
-/// wrong. Every formula of the spreadsheets in the corpus is evaluated, and
-/// each answer is compared with the result the file caches.
+/// Compare supported formulas with cached results (spreadsheet-editing decision
+/// 29).
 TEST(FormulaCorpus, every_answer_is_the_result_the_file_caches) {
   Tally tally;
   for (const FileType type : {FileType::opendocument_spreadsheet,
@@ -140,6 +139,11 @@ TEST(FormulaCorpus, every_answer_is_the_result_the_file_caches) {
     }
   }
 
+  if (tally.formulas == 0) {
+    GTEST_SKIP() << "No formula-bearing corpus files available";
+  }
+  ASSERT_GT(tally.answered, 0u) << "No cached formula results were checked";
+
   std::cout << tally.answered << " of " << tally.formulas
             << " formulas answered, " << tally.differences.size()
             << " differ from the cached result" << std::endl;

test/src/odr_test.cpp

@@ -246,88 +246,95 @@ TEST(FileTypeTable, capabilities_build_on_each_other) {
 
 /// Holds the declared `color_scheme` against what the renderer emits.
 TEST(FileTypeCapabilities, color_scheme_matches_the_html) {
-  const auto &logger = Logger::null();
-
+  std::size_t candidates = 0;
   for (const FileType type : every_file_type()) {
     const FileTypeCapabilities declared = capabilities_by_file_type(type);
     if (!declared.translate_html) {
       continue;
     }
-
-    const std::vector<TestFile> test_files = TestData::test_files(type);
-    if (test_files.empty() || test_files.front().password.has_value()) {
-      continue;
+    const std::size_t before = candidates;
+    bool checked = false;
+    std::string error;
+    for (const TestFile &test_file : TestData::test_files(type)) {
+      if (test_file.password.has_value()) {
+        continue;
+      }
+      ++candidates;
+      try {
+        const DecodedFile file =
+            open(test_file.absolute_path, DecodeOptions::as(type));
+        const auto render = [&](const HtmlColorScheme scheme) {
+          HtmlConfig config;
+          config.color_scheme = scheme;
+          // one page of a pdf is enough, and the rest is slow
+          config.page_range_end = 1;
+          std::ostringstream out;
+          html::translate(file, config).list_views().at(0).write_html(out);
+          return out.str();
+        };
+        EXPECT_EQ(render(HtmlColorScheme::light) !=
+                      render(HtmlColorScheme::dark),
+                  declared.color_scheme)
+            << test_file.short_path;
+        checked = true;
+        break;
+      } catch (const std::exception &failure) {
+        error = test_file.short_path + ": " + failure.what();
+      }
     }
-
-    std::optional<DecodedFile> file;
-    try {
-      file = open(test_files.front().absolute_path, DecodeOptions::as(type),
-                  logger);
-    } catch (...) {
-      continue;
+    if (candidates != before) {
+      EXPECT_TRUE(checked) << file_type_to_string(type) << ": " << error;
     }
-
-    const auto render = [&](const HtmlColorScheme scheme) {
-      HtmlConfig config;
-      config.color_scheme = scheme;
-      // one page of a pdf is enough, and the rest is slow
-      config.page_range_end = 1;
-
-      std::ostringstream out;
-      html::translate(*file, config).list_views().at(0).write_html(out);
-      return out.str();
-    };
-
-    std::string light;
-    std::string dark;
-    try {
-      light = render(HtmlColorScheme::light);
-      dark = render(HtmlColorScheme::dark);
-    } catch (...) {
-      continue;
-    }
-
-    EXPECT_EQ(light != dark, declared.color_scheme)
-        << file_type_to_string(type) << " " << test_files.front().short_path;
+  }
+  if (candidates == 0) {
+    GTEST_SKIP() << "No unencrypted corpus files available for rendering";
   }
 }
 
-/// The anti-drift check behind `capabilities_by_file_type`: what the engines
-/// actually do must never exceed what the table declares. Capabilities are
-/// per-format constants today, so a couple of files per type is enough.
+/// Engine capabilities must stay within the format's declarations.
 TEST(FileTypeCapabilities, declaration_matches_the_engines) {
-  static constexpr std::size_t files_per_file_type = 2;
-
-  const auto &logger = Logger::null();
-
+  constexpr std::size_t files_per_file_type = 2;
+  std::size_t candidates = 0;
   for (const FileType type : every_file_type()) {
     const FileTypeCapabilities declared = capabilities_by_file_type(type);
-    const std::vector<TestFile> test_files = TestData::test_files(type);
-
-    const std::size_t count = std::min(files_per_file_type, test_files.size());
-    for (std::size_t i = 0; i < count; ++i) {
-      const TestFile &test_file = test_files[i];
-
+    const std::size_t before = candidates;
+    std::size_t checked = 0;
+    std::string error;
+    for (const TestFile &test_file : TestData::test_files(type)) {
+      if (checked == files_per_file_type) {
+        break;
+      }
+      if ((declared.edit || declared.save) && test_file.password.has_value()) {
+        continue;
+      }
+      ++candidates;
       if (!declared.open) {
         EXPECT_ANY_THROW(std::ignore = open(test_file.absolute_path,
-                                            DecodeOptions::as(type), logger))
+                                            DecodeOptions::as(type)))
             << test_file.short_path;
+        ++checked;
         continue;
       }
 
       std::optional<DecodedFile> file;
+      std::optional<Document> document;
       try {
-        file = open(test_file.absolute_path, DecodeOptions::as(type), logger);
-      } catch (...) {
-        // declared support is an upper bound — a single file may still fail
+        file = open(test_file.absolute_path, DecodeOptions::as(type));
+        if (file->is_document_file() && !file->password_encrypted()) {
+          document = file->as_document_file().document();
+        }
+      } catch (const std::exception &failure) {
+        // declared support is an upper bound: one file may fail, as long as
+        // another of its type passes
+        error = test_file.short_path + ": " + failure.what();
         continue;
       }
-
-      // whatever detection sees, the table has to admit to — from the bytes,
+      ++checked;
+      // whatever detection sees, the table has to admit to - from the bytes,
       // or from the name for a type that has no signature to find
       const std::vector<FileType> detected =
-          list_file_types(test_file.absolute_path, logger);
-      if (std::ranges::find(detected, type) != std::ranges::end(detected)) {
+          list_file_types(test_file.absolute_path);
+      if (std::ranges::find(detected, type) != detected.end()) {
         EXPECT_TRUE(declared.detect_by_content ||
                     file_type_by_file_extension(
                         Path(test_file.absolute_path).extension()) == type)
@@ -343,31 +350,26 @@ TEST(FileTypeCapabilities, declaration_matches_the_engines) {
           << test_file.short_path;
       EXPECT_TRUE(!actual.translate_html || declared_actual.translate_html)
           << test_file.short_path;
-
-      // A plain file is not a document, so `Document::is_savable` never
-      // answers for it - and without this the table could claim anything.
+      // a plain file is not a document, so `Document::is_savable` never
+      // answers for it, and without this the table could claim anything
       if (file->is_text_file()) {
         EXPECT_EQ(file->as_text_file().is_savable(), declared_actual.save)
             << test_file.short_path;
       }
-
-      if (!file->is_document_file() || file->password_encrypted()) {
-        continue;
-      }
-
-      std::optional<Document> document;
-      try {
-        document = file->as_document_file().document();
-      } catch (...) {
-        continue;
+      if (document.has_value()) {
+        EXPECT_EQ(document->is_editable(), declared_actual.edit)
+            << test_file.short_path;
+        EXPECT_EQ(document->is_savable(false), declared_actual.save)
+            << test_file.short_path;
+        EXPECT_EQ(document->is_savable(true), declared_actual.encrypt)
+            << test_file.short_path;
       }
-
-      EXPECT_EQ(document->is_editable(), declared_actual.edit)
-          << test_file.short_path;
-      EXPECT_EQ(document->is_savable(false), declared_actual.save)
-          << test_file.short_path;
-      EXPECT_EQ(document->is_savable(true), declared_actual.encrypt)
-          << test_file.short_path;
     }
+    if (candidates != before) {
+      EXPECT_GT(checked, 0u) << file_type_to_string(type) << ": " << error;
+    }
+  }
+  if (candidates == 0) {
+    GTEST_SKIP() << "No corpus files available for capability checks";
   }
 }