diff --git a/.agents/skills/testing-doc-pdf/SKILL.md b/.agents/skills/testing-doc-pdf/SKILL.md index 9fb33feeb..5dde9bb8b 100644 --- a/.agents/skills/testing-doc-pdf/SKILL.md +++ b/.agents/skills/testing-doc-pdf/SKILL.md @@ -15,7 +15,7 @@ description: How to end-to-end test the sysml PDF document backend (internal/doc - `OPENSYSML_KATEX=$PWD/build/doc-pdf/katex/node_modules/.bin/katex` (only a document with formulas needs it) - `OPENSYSML_DOT=$PWD/build/doc-pdf/graphviz/bin/dot` (for `-diagram-form dot`; absent, DOT blocks stay as source under a notice) - `OPENSYSML_PLANTUML_JAR=$PWD/build/doc-pdf/plantuml/plantuml-1.2026.8.jar` (for `-diagram-form plantuml`; absent, PlantUML blocks stay as source under a notice) -- `go test -run Installed ./internal/doc/docpdf` runs the real-toolchain integration tests (they skip per missing tool); the rest of the package's tests use fake tools and need nothing installed. `OPENSYSML_REQUIRE_PDF_TOOLCHAIN=1` turns every such skip but Prince's into a failure — set it once the script has run, so a misconfigured variable cannot pass as a skip. This is how the CI `pdf-toolchain` job runs them. +- `go test -run Installed ./internal/doc/docpdf` runs the real-toolchain integration tests (they skip per missing tool, `pdftotext`/`pdfimages` from `poppler-utils` included — the tests read every PDF back through them, so install the package or the text assertions never run); the rest of the package's tests use fake tools and need nothing installed. `OPENSYSML_REQUIRE_PDF_TOOLCHAIN=1` turns every such skip but Prince's into a failure — set it once the script has run, so a misconfigured variable cannot pass as a skip. This is how the CI `pdf-toolchain` job runs them. - Diagram forms: render the worked example with `-diagram-form dot` and `-diagram-form plantuml` too; the PDF must contain no "did not draw" notice (pypdf `extract_text`) and the page's XObjects grow by one per diagram. With `OPENSYSML_DOT=/nonexistent` the render still succeeds and the notice names `OPENSYSML_DOT`. A `// layout: neato -n` block (the DOT writer emits one for a positioned diagram) must run neato: `dot -Kneato` failing with "no layout engine support" means the GTS library is missing. ## Rendering diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 036f2e408..001ce9359 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -671,13 +671,14 @@ jobs: key: doc-pdf-toolchain-${{ runner.os }}-${{ hashFiles('scripts/download-doc-pdf-toolchain.sh') }}-py${{ steps.setup-python.outputs.python-version }} # WeasyPrint's venv links against the system's Pango, the browser - # mermaid-cli launches needs its shared libraries, and Graphviz's neato - # layout plugin needs GTS. + # mermaid-cli launches needs its shared libraries, Graphviz's neato + # layout plugin needs GTS; the tests read PDFs back with poppler-utils. - name: Install the toolchain's system libraries run: | sudo apt-get update sudo apt-get install -y --no-install-recommends libpango-1.0-0 libpangoft2-1.0-0 libharfbuzz-subset0 \ - libnss3 libatk-bridge2.0-0 libgbm1 libxkbcommon0 libasound2t64 fonts-dejavu-core libgts-0.7-5 + libnss3 libatk-bridge2.0-0 libgbm1 libxkbcommon0 libasound2t64 fonts-dejavu-core libgts-0.7-5 \ + poppler-utils - name: Provision the PDF toolchain run: ./scripts/download-doc-pdf-toolchain.sh diff --git a/changes/unreleased/pdf-toolchain-poppler.fixed.md b/changes/unreleased/pdf-toolchain-poppler.fixed.md new file mode 100644 index 000000000..a757bd756 --- /dev/null +++ b/changes/unreleased/pdf-toolchain-poppler.fixed.md @@ -0,0 +1 @@ +- **The PDF toolchain CI job now reads its rendered PDFs back.** The job installs `poppler-utils`, and the integration tests treat an absent `pdftotext`/`pdfimages` like an absent converter: a skip locally, a failure under `OPENSYSML_REQUIRE_PDF_TOOLCHAIN`. Before, the text and image assertions (headings, captions, formulas, diagram source kept off the page) silently skipped in CI for want of `pdftotext`. diff --git a/internal/doc/docpdf/integration_test.go b/internal/doc/docpdf/integration_test.go index 81d9cb219..47a7be432 100644 --- a/internal/doc/docpdf/integration_test.go +++ b/internal/doc/docpdf/integration_test.go @@ -56,7 +56,7 @@ func installedConverter(t *testing.T, engine string) Converter { } // renderInstalled renders document with an installed engine and returns the -// PDF's text as pdftotext extracts it, skipping when pdftotext is absent. +// PDF's text as pdftotext extracts it. func renderInstalled(t *testing.T, document *docir.Document, engine string, opts Options) (pdf []byte, text string) { t.Helper() installedConverter(t, engine) @@ -70,12 +70,13 @@ func renderInstalled(t *testing.T, document *docir.Document, engine string, opts return pdf, pdfText(t, pdf) } -// pdfText extracts a PDF's text with pdftotext, "" when it is not installed. +// pdfText extracts a PDF's text with pdftotext; absent, it skips the test, or +// fails it when the toolchain is mandatory. func pdfText(t *testing.T, pdf []byte) string { t.Helper() pdftotext, err := exec.LookPath("pdftotext") if err != nil { - return "" + skipWithout(t, "pdftotext", err) } dir := t.TempDir() if err := os.WriteFile(filepath.Join(dir, "doc.pdf"), pdf, 0o600); err != nil { @@ -89,12 +90,12 @@ func pdfText(t *testing.T, pdf []byte) string { } // pdfImages lists a PDF's raster images as pdfimages reports them, one line -// each, "" when it is not installed. +// each; an absent pdfimages is handled as in pdfText. func pdfImages(t *testing.T, pdf []byte) string { t.Helper() pdfimages, err := exec.LookPath("pdfimages") if err != nil { - return "" + skipWithout(t, "pdfimages", err) } dir := t.TempDir() if err := os.WriteFile(filepath.Join(dir, "doc.pdf"), pdf, 0o600); err != nil { @@ -130,16 +131,10 @@ func TestRenderStylesheetAssetsBesideTheOutput(t *testing.T) { for _, engine := range Engines() { t.Run(engine, func(t *testing.T) { pdf, text := renderInstalled(t, document, engine, Options{Stylesheets: []docrender.Stylesheet{sheet}, BaseDir: out}) - if text == "" { - t.Skip("pdftotext not installed") - } if !strings.Contains(text, "IMPORTEDBESIDE") { t.Errorf("the imported sheet beside the PDF did not apply:\n%s", text) } images := pdfImages(t, pdf) - if images == "" { - t.Skip("pdfimages not installed") - } if !regexp.MustCompile(`(?m)^\s*1\s+0\s+image\s+12\s+12\s`).MatchString(images) { t.Errorf("the image beside the PDF was not drawn:\n%s", images) } @@ -154,7 +149,7 @@ func TestRenderWithInstalledEngines(t *testing.T) { for _, engine := range Engines() { t.Run(engine, func(t *testing.T) { _, text := renderInstalled(t, document, engine, Options{TOC: true, NumberSections: true}) - if text != "" && !strings.Contains(text, "One paragraph.") { + if !strings.Contains(text, "One paragraph.") { t.Fatalf("paragraph missing from the PDF text:\n%s", text) } }) @@ -174,9 +169,6 @@ func TestRenderTelescopeWithInstalledEngines(t *testing.T) { for _, engine := range Engines() { t.Run(engine, func(t *testing.T) { _, text := renderInstalled(t, document, engine, Options{TitlePage: true, TOC: true, NumberSections: true}) - if text == "" { - t.Skip("pdftotext not installed") - } for _, want := range []string{ "Telescope Mass Report", "Subsystems grouped by zone", @@ -251,9 +243,6 @@ func TestRenderInlineRunsWithInstalledEngines(t *testing.T) { for _, engine := range Engines() { t.Run(engine, func(t *testing.T) { _, text := renderInstalled(t, document, engine, Options{}) - if text == "" { - t.Skip("pdftotext not installed") - } for _, want := range []string{"The margin is critical for m > 0 per the spec", "See Subsystems by zone below.", "zone: hot", "zone: cold", "mirror"} { if !strings.Contains(text, want) { t.Errorf("PDF text lacks %q:\n%s", want, text) @@ -308,9 +297,6 @@ func TestRenderFormulasWithInstalledKatex(t *testing.T) { for _, engine := range Engines() { t.Run(engine, func(t *testing.T) { _, text := renderInstalled(t, document, engine, Options{TOC: true}) - if text == "" { - t.Skip("pdftotext not installed") - } for _, want := range []string{"Collecting area of a circular mirror", "Rayleigh criterion", "each $ of budget"} { if !strings.Contains(text, want) { t.Errorf("PDF text lacks %q:\n%s", want, text) @@ -333,9 +319,6 @@ func TestRenderStateReportWithInstalledEngines(t *testing.T) { for _, engine := range Engines() { t.Run(engine, func(t *testing.T) { _, text := renderInstalled(t, document, engine, Options{TOC: true}) - if text == "" { - t.Skip("pdftotext not installed") - } for _, want := range []string{ "Lamp Report", "Active states of every lamp", @@ -365,7 +348,7 @@ func TestRenderDiagramsWithInstalledMermaid(t *testing.T) { skipWithout(t, "mmdc", err) } _, text := renderInstalled(t, telescopeDocument(t), "", Options{}) - if text != "" && !strings.Contains(text, "Imaging chain interconnection") { + if !strings.Contains(text, "Imaging chain interconnection") { t.Fatalf("diagram caption missing:\n%s", text) } } @@ -422,10 +405,10 @@ func TestRenderDiagramsWithInstalledGraphviz(t *testing.T) { } _, text := renderInstalled(t, telescopeDocument(t), "", Options{DiagramForm: view.FormDot}) - if text != "" && (strings.Contains(text, "digraph") || strings.Contains(text, dotNotice[:40])) { + if strings.Contains(text, "digraph") || strings.Contains(text, dotNotice[:40]) { t.Fatalf("DOT source or its notice reached the PDF:\n%s", text) } - if text != "" && !strings.Contains(text, "Imaging chain interconnection") { + if !strings.Contains(text, "Imaging chain interconnection") { t.Fatalf("diagram caption missing:\n%s", text) } } @@ -466,10 +449,10 @@ func TestRenderDiagramsWithInstalledPlantUML(t *testing.T) { } _, text := renderInstalled(t, telescopeDocument(t), "", Options{DiagramForm: view.FormPlantUML}) - if text != "" && (strings.Contains(text, "@startuml") || strings.Contains(text, plantumlNotice[:40])) { + if strings.Contains(text, "@startuml") || strings.Contains(text, plantumlNotice[:40]) { t.Fatalf("PlantUML source or its notice reached the PDF:\n%s", text) } - if text != "" && !strings.Contains(text, "Imaging chain interconnection") { + if !strings.Contains(text, "Imaging chain interconnection") { t.Fatalf("diagram caption missing:\n%s", text) } }