diff --git a/.claude/rules/testing/unit-test-with-r.md b/.claude/rules/testing/unit-test-with-r.md index c826b0e544c..65207716172 100644 --- a/.claude/rules/testing/unit-test-with-r.md +++ b/.claude/rules/testing/unit-test-with-r.md @@ -1,15 +1,18 @@ --- paths: - "tests/unit/preview-subdir-knitr-cwd.test.ts" + - "tests/smoke/embed/render-embed-website-rerender.test.ts" --- -# R unit test that changes working directory +# R tests that change working directory -This test runs a knitr subprocess with cwd set to a scratch directory -(required to reproduce the bug). R's `.Rprofile` lookup is cwd-exact, so it -won't pick up `tests/renv` activation there — on CI the R subprocess fails -with `there is no package called 'rmarkdown'`. `setup()` writes a +These tests run a knitr subprocess with cwd set to a scratch or fixture +directory (required to reproduce the bug). R's `.Rprofile` lookup is +cwd-exact, so it won't pick up `tests/renv` activation there — on CI the R +subprocess fails with `there is no package called 'rmarkdown'`. `setup()` +calls `writeTestsRenvProfile(dir)` (`tests/utils.ts`), which writes a `.Rprofile` into the fixture dir to re-activate renv against the real -`tests/` project. +`tests/` project. Any new test whose knitr cwd is outside `tests/` needs the +same call. **Details:** `llm-docs/testing-patterns.md` → "R Tests That Change Working Directory" diff --git a/llm-docs/testing-patterns.md b/llm-docs/testing-patterns.md index 31d250ad9bb..db763ce3f80 100644 --- a/llm-docs/testing-patterns.md +++ b/llm-docs/testing-patterns.md @@ -218,6 +218,7 @@ unitTest("runs from workingDir", async () => { **Key points:** +- A test that runs R/knitr from the new cwd also needs the renv activation described in "R Tests That Change Working Directory" below. - The harness calls `cwd()` **before** `setup()`, so the directory must already exist when `cwd()` runs — create it at module scope, not in `setup`. - `teardown` runs **before** the harness restores the cwd, so on Windows the temp dir may still be the cwd and resist removal. Wrap the removal in try/catch (best-effort) — see `tests/smoke/use/template.test.ts` and `tests/unit/dotenv-config.test.ts`. @@ -429,7 +430,7 @@ Most knitr tests never leave `tests/` — they pass paths relative to the curren A test that changes cwd through `TestContext.cwd()` loses that activation. The R subprocess starts outside `tests/`, and package loads may fail with `there is no package called 'rmarkdown'`. A developer machine with rmarkdown on the default `.libPaths()` may mask this CI failure. -**Fix:** write a `.Rprofile` in the fixture cwd that points renv to the test project: +**Fix:** call `writeTestsRenvProfile(dir)` (`tests/utils.ts`) in `setup()` to write a `.Rprofile` in the fixture cwd that points renv to the test project. It writes: ```r Sys.setenv(RENV_PROJECT = "") diff --git a/news/changelog-1.11.md b/news/changelog-1.11.md index e9da3f5f958..ca188754335 100644 --- a/news/changelog-1.11.md +++ b/news/changelog-1.11.md @@ -65,6 +65,7 @@ All changes included in 1.11: ### Websites +- ([#10756](https://github.com/quarto-dev/quarto-cli/issues/10756)): Fix the notebook preview of an embedded `.qmd` file going missing from the output directory when a project is rendered again, which broke its source notebook link. - ([#14974](https://github.com/quarto-dev/quarto-cli/issues/14974)): Fix math in `.llms.md` files from `llms-txt` coming out garbled with the default `mathjax` method, and with `katex` and `webtex`. Math is now written as `$...$` and `$$...$$` with the `mathjax`, `katex`, `webtex` and `mathml` methods. ## Lua API diff --git a/src/render/notebook/notebook-context.ts b/src/render/notebook/notebook-context.ts index a7565ef061c..972fd5fa7d2 100644 --- a/src/render/notebook/notebook-context.ts +++ b/src/render/notebook/notebook-context.ts @@ -77,7 +77,7 @@ export function notebookContext(): NotebookContext { notebooks[nbAbsPath] = nb; needRewrite = true; - if (context) { + if (context && !cached) { const contrib = contributor(renderType); if (contrib.cache) { contrib.cache(output, context); @@ -289,19 +289,32 @@ export function notebookContext(): NotebookContext { // If there is a source representation of the qmd file // we should use that, which will prevent rexecution of the // QMD - const notebook = notebooks[nbAbsPath]; - const toRenderPath = notebook - ? notebook[kQmdIPynb] ? notebook[kQmdIPynb].path : nbAbsPath + const qmdIpynb = notebooks[nbAbsPath]?.[kQmdIPynb]; + // A notebook revived from the scratch cache is rendered from beside the + // source, so the preview and its files land where a fresh render puts them + const staged = qmdIpynb?.cached === true; + const toRenderPath = qmdIpynb + ? staged ? qmdIpynb.hrefPath : qmdIpynb.path : nbAbsPath; + if (staged) { + Deno.copyFileSync(qmdIpynb.path, qmdIpynb.hrefPath); + } - const renderedFile = await contributor(renderType).render( - toRenderPath, - format, - token(), - services, - notebookMetadata, - project, - ); + let renderedFile: NotebookRenderResult; + try { + renderedFile = await contributor(renderType).render( + toRenderPath, + format, + token(), + services, + notebookMetadata, + project, + ); + } finally { + if (staged) { + safeRemoveIfExists(toRenderPath); + } + } addRendering(nbAbsPath, renderType, renderedFile, project); if (!notebooks[nbAbsPath][renderType]) { diff --git a/tests/docs/embed/website-rerender/.gitignore b/tests/docs/embed/website-rerender/.gitignore new file mode 100644 index 00000000000..2b7a027edff --- /dev/null +++ b/tests/docs/embed/website-rerender/.gitignore @@ -0,0 +1,5 @@ +/.quarto/ +/_site/ + +**/*.quarto_ipynb +/.Rprofile diff --git a/tests/docs/embed/website-rerender/_quarto.yml b/tests/docs/embed/website-rerender/_quarto.yml new file mode 100644 index 00000000000..797a825157e --- /dev/null +++ b/tests/docs/embed/website-rerender/_quarto.yml @@ -0,0 +1,4 @@ +project: + type: website + +format: html diff --git a/tests/docs/embed/website-rerender/index.qmd b/tests/docs/embed/website-rerender/index.qmd new file mode 100644 index 00000000000..5da06619e81 --- /dev/null +++ b/tests/docs/embed/website-rerender/index.qmd @@ -0,0 +1,5 @@ +--- +title: "Home" +--- + +{{< embed source.qmd#fig-cars >}} diff --git a/tests/docs/embed/website-rerender/source.qmd b/tests/docs/embed/website-rerender/source.qmd new file mode 100644 index 00000000000..77869c40b3c --- /dev/null +++ b/tests/docs/embed/website-rerender/source.qmd @@ -0,0 +1,9 @@ +--- +title: "Source" +--- + +```{r} +#| label: fig-cars +#| fig-cap: "Cars" +plot(cars) +``` diff --git a/tests/docs/site/.gitignore b/tests/docs/site/.gitignore index 075b2542afb..0e3521a7d0f 100644 --- a/tests/docs/site/.gitignore +++ b/tests/docs/site/.gitignore @@ -1 +1,3 @@ /.quarto/ + +**/*.quarto_ipynb diff --git a/tests/smoke/embed/render-embed-website-rerender.test.ts b/tests/smoke/embed/render-embed-website-rerender.test.ts new file mode 100644 index 00000000000..aa7b8ac431f --- /dev/null +++ b/tests/smoke/embed/render-embed-website-rerender.test.ts @@ -0,0 +1,91 @@ +/* + * render-embed-website-rerender.test.ts + * + * Copyright (C) 2026 Posit Software, PBC + */ + +import { join, resolve } from "../../../src/deno_ral/path.ts"; +import { existsSync, safeRemoveSync } from "../../../src/deno_ral/fs.ts"; +import { Element } from "../../../src/core/deno-dom.ts"; +import { docs, writeTestsRenvProfile } from "../../utils.ts"; +import { + ensureHtmlSelectorSatisfies, + fileExists, + noErrors, + pathDoNotExists, +} from "../../verify.ts"; +import { test } from "../../test.ts"; +import { runQuarto } from "../../quarto-cmd.ts"; + +// A re-render of a website must keep the notebook preview of an embedded +// qmd, even though the embed notebook is now served from the project +// scratch cache (#10756) +const projectDir = resolve(docs("embed/website-rerender")); +const siteDir = join(projectDir, "_site"); + +const removeGenerated = () => { + for ( + const name of [ + "_site", + ".quarto", + "index_files", + "source.embed_files", + "source_files", + "source.embed.ipynb", + "source.embed-preview.html", + ".Rprofile", + ] + ) { + safeRemoveSync(join(projectDir, name), { recursive: true }); + } +}; + +test({ + name: "embed qmd preview survives a second website render (#10756)", + context: { + // Rendering from outside the project loses the preview regardless of + // this fix (#15004), so run from the project directory + cwd: () => projectDir, + setup: () => { + removeGenerated(); + // knitr runs from the project dir, outside tests/ + writeTestsRenvProfile(projectDir); + return Promise.resolve(); + }, + teardown: () => { + removeGenerated(); + return Promise.resolve(); + }, + }, + execute: async (logFile?: string) => { + await runQuarto(["render", projectDir], { logFile, throwOnFailure: false }); + await runQuarto(["render", projectDir], { logFile, throwOnFailure: false }); + }, + verify: [ + noErrors, + fileExists(join(siteDir, "source.embed-preview.html")), + // download target of the "Source" link + fileExists(join(siteDir, "source.qmd")), + // every figure in the preview resolves to a non-empty file in _site + ensureHtmlSelectorSatisfies( + join(siteDir, "source.embed-preview.html"), + "img.figure-img", + (nodeList) => { + const srcs = Array.from(nodeList).map((n) => + (n as Element).getAttribute("src") + ); + return srcs.length > 0 && srcs.every((src) => + src !== null && existsSync(join(siteDir, src)) && + Deno.statSync(join(siteDir, src)).size > 0 + ); + }, + ), + // figure files were never relocated under the scratch dir + pathDoNotExists(join(siteDir, ".quarto")), + // revived notebook isn't re-cached into a nested scratch dir + pathDoNotExists(join(projectDir, ".quarto", "embed", ".quarto")), + // staged notebook is removed after rendering the preview + pathDoNotExists(join(projectDir, "source.embed.ipynb")), + ], + type: "smoke", +}); diff --git a/tests/unit/preview-subdir-knitr-cwd.test.ts b/tests/unit/preview-subdir-knitr-cwd.test.ts index cc249651c15..c8763d19b71 100644 --- a/tests/unit/preview-subdir-knitr-cwd.test.ts +++ b/tests/unit/preview-subdir-knitr-cwd.test.ts @@ -29,12 +29,8 @@ import { unitTest } from "../test.ts"; import { assert } from "testing/asserts"; -import { - dirname, - fromFileUrl, - isAbsolute, - join, -} from "../../src/deno_ral/path.ts"; +import { dirname, isAbsolute, join } from "../../src/deno_ral/path.ts"; +import { writeTestsRenvProfile } from "../utils.ts"; import { existsSync } from "../../src/deno_ral/fs.ts"; import { which } from "../../src/core/path.ts"; import { projectContext } from "../../src/project/project-context.ts"; @@ -160,13 +156,7 @@ unitTest( // Re-activate renv against the real tests/ project, regardless of // this fixture's cwd — see llm-docs/testing-patterns.md → "R Tests // That Change Working Directory" for why this is needed. - const testsDir = dirname(dirname(fromFileUrl(import.meta.url))) - .replaceAll("\\", "/"); - Deno.writeTextFileSync( - join(e2eProjDir, ".Rprofile"), - `Sys.setenv(RENV_PROJECT = "${testsDir}")\n` + - `source("${testsDir}/renv/activate.R")\n`, - ); + writeTestsRenvProfile(e2eProjDir); Deno.writeTextFileSync( join(e2eProjDir, "_quarto.yml"), diff --git a/tests/utils.ts b/tests/utils.ts index a3a9473febb..dd01ff6b18f 100644 --- a/tests/utils.ts +++ b/tests/utils.ts @@ -5,7 +5,7 @@ * */ -import { basename, dirname, extname, join, relative } from "../src/deno_ral/path.ts"; +import { basename, dirname, extname, fromFileUrl, join, relative } from "../src/deno_ral/path.ts"; import { parseFormatString } from "../src/core/pandoc/pandoc-formats.ts"; import { kMetadataFormat, kOutputExt, kOutputFile } from "../src/config/constants.ts"; import { pathWithForwardSlashes, safeExistsSync } from "../src/core/path.ts"; @@ -244,6 +244,19 @@ export function docs(path: string): string { return join("docs", path); } +// Write a `.Rprofile` into `dir` that activates the tests/ renv project. +// R reads `.Rprofile` from the exact cwd only, so an R subprocess started +// outside tests/ (a test using `TestContext.cwd`) would not see the renv +// library holding knitr/rmarkdown on CI. +export function writeTestsRenvProfile(dir: string) { + const testsDir = pathWithForwardSlashes(dirname(fromFileUrl(import.meta.url))); + Deno.writeTextFileSync( + join(dir, ".Rprofile"), + `Sys.setenv(RENV_PROJECT = "${testsDir}")\n` + + `source("${testsDir}/renv/activate.R")\n`, + ); +} + export function fileLoader(...path: string[]) { return (file: string, to: string) => { const input = docs(join(...path, file));