Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .claude/rules/testing/test-anti-patterns.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,9 @@ paths:
Never create `Project.toml`, `.venv/`, or `renv.lock` in test fixture directories.

**Details:** `llm-docs/testing-patterns.md` → "Shared Test Environments"

## Don't: `Deno.chdir()` inside the test body

`Deno.chdir()` mutates process-global cwd, so a test that changes it can leak into other tests in the same process. The harness already changes and restores the working directory: return the directory from `TestContext.cwd`, create fixtures in `setup`, clean up in `teardown` (examples: `tests/unit/dotenv-config.test.ts`, `tests/smoke/use/template.test.ts`). For a temp directory you don't need to run *from*, use `withTempDir` (`tests/utils.ts`). A test that only needs a *relative* input can pass a path relative to the current cwd without changing it.

**Details:** `llm-docs/testing-patterns.md` → "Working-Directory-Sensitive Tests"
15 changes: 15 additions & 0 deletions .claude/rules/testing/unit-test-with-r.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
paths:
- "tests/unit/preview-subdir-knitr-cwd.test.ts"
---

# R unit test that changes 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
`.Rprofile` into the fixture dir to re-activate renv against the real
`tests/` project.

**Details:** `llm-docs/testing-patterns.md` → "R Tests That Change Working Directory"
64 changes: 64 additions & 0 deletions llm-docs/testing-patterns.md
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,40 @@ testQuartoCmd(
- Clean up entire temp directory including source files
- File verification uses relative paths when checking files in `cwd()`

### Working-Directory-Sensitive Tests

Some tests need to run from a specific directory (e.g. reproducing a bug that
depends on the process cwd). **Do not `Deno.chdir()` inside the test body** —
it mutates process-global cwd and can leak into other tests in the same
process. Use the `TestContext` options instead; the harness changes the cwd
before the test and restores it afterward:

```typescript
const workingDir = Deno.makeTempDirSync();

unitTest("runs from workingDir", async () => {
// cwd is workingDir here (set by the harness)
}, {
setup: () => { Deno.writeTextFileSync(".env.example", "..."); return Promise.resolve(); },
cwd: () => workingDir,
teardown: () => { try { Deno.removeSync(workingDir, { recursive: true }); } catch { /* best-effort */ } return Promise.resolve(); },
});
```

**Key points:**

- 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`.
- For a temp directory you don't need to run *from*, prefer `withTempDir`
(`tests/utils.ts`), which creates and recursively removes it in a `finally`.
- A test that only needs a **relative** input (not a specific cwd) can pass a
path relative to the current cwd (`relative(Deno.cwd(), absFile)`) without
changing directories at all.

## Verification Helpers

### Core Verifiers
Expand Down Expand Up @@ -363,6 +397,36 @@ Rscript -e "renv::install(); renv::snapshot()"

**Note:** While Quarto supports local Project.toml files in document directories for production use, the quarto-cli test infrastructure specifically does NOT support this pattern. All test dependencies must be in the main `tests/` environment.

### R Tests That Change Working Directory

R resolves `.Rprofile` from the **exact** process cwd (no parent-directory
search). On CI, rmarkdown/knitr live only in `tests/renv`'s project library,
activated when cwd is `tests/` (via `tests/.Rprofile` sourcing
`renv/activate.R`). Most knitr tests never leave `tests/` — they pass paths
relative to the current cwd instead of changing directories — so activation
happens automatically.

A test that must run with cwd set elsewhere (a scratch temp dir, via
`TestContext.cwd()` — see "Working-Directory-Sensitive Tests" above) loses
that activation: the R subprocess starts outside `tests/`, renv never
activates, and package loads fail with `there is no package called
'rmarkdown'`. This is CI-only — a developer machine with rmarkdown on the
default `.libPaths()` masks it entirely. The render pipeline also tends to
swallow the underlying subprocess error, so the failure can be silent beyond
the bare package-load message.

**Fix:** in the fixture's cwd, write a `.Rprofile` that re-points renv at the
real project, regardless of where the test's cwd actually is:

```r
Sys.setenv(RENV_PROJECT = "<absolute path to tests/>")
source("<absolute path to tests/>/renv/activate.R")
```

`renv/activate.R` reads `RENV_PROJECT` to determine the project root if set,
falling back to `getwd()` otherwise — setting it explicitly decouples renv
activation from the test's cwd.

## Best Practices

1. **Always clean up**: Use teardown to remove generated files
Expand Down
2 changes: 2 additions & 0 deletions news/changelog-1.10.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ All changes included in 1.10:
- ([#14281](https://github.com/quarto-dev/quarto-cli/issues/14281)): Fix transient `.quarto_ipynb` files accumulating during `quarto preview` with Jupyter engine.
- ([#14298](https://github.com/quarto-dev/quarto-cli/issues/14298)): Fix `quarto preview` browse URL including output filename (e.g., `hello.html`) for single-file documents, breaking Posit Workbench proxied server access.
- ([#14489](https://github.com/quarto-dev/quarto-cli/issues/14489)): Restore `--output-dir` support for `quarto preview` of single files when no `_quarto.yml` is present (e.g. R-package workspaces). Regression introduced in v1.9.18.
- ([#14683](https://github.com/quarto-dev/quarto-cli/issues/14683)): Fix `quarto preview` of a knitr document in a project subdirectory failing when run from that subdirectory with a bare filename (the shape RStudio's Render button uses). Regression introduced in v1.9.18.
- ([rstudio/rstudio#17333](https://github.com/rstudio/rstudio/issues/17333)): Fix `quarto inspect` on standalone files emitting project metadata that breaks RStudio's publishing wizard.

## Dependencies
Expand Down Expand Up @@ -108,6 +109,7 @@ All changes included in 1.10:
- ([#6092](https://github.com/quarto-dev/quarto-cli/issues/6092)): Fix the `default-image-extension` being appended to an extensionless URL ending in a slash (e.g. `![](https://example.com/)`), which broke the embed/iframe image syntax.
- ([#6651](https://github.com/quarto-dev/quarto-cli/issues/6651)): Fix dart-sass compilation failing in enterprise environments where `.bat` files are blocked by group policy.
- ([#11703](https://github.com/quarto-dev/quarto-cli/issues/11703)): Fix SCSS color-variable export (`--quarto-scss-export-*`) being silently skipped when a theme rule contains consecutive semicolons (e.g. `max-height: 100% !important;;`).
- ([#12401](https://github.com/quarto-dev/quarto-cli/issues/12401)): Fix `QUARTO_DOCUMENT_PATH` being a relative path for single-file renders, inconsistent with project renders where it was absolute. Execution target paths are now normalized to absolute at a single point, so single-file and project renders behave the same.
- ([#14255](https://github.com/quarto-dev/quarto-cli/issues/14255)): Fix shortcodes inside inline and display math expressions not being resolved.
- ([#14342](https://github.com/quarto-dev/quarto-cli/issues/14342)): Work around TOCTOU race in Deno's `expandGlobSync` that can cause unexpected exceptions to be raised while traversing directories during project initialization.
- ([#14445](https://github.com/quarto-dev/quarto-cli/issues/14445)): Fix intermittent `Uncaught (in promise) TypeError: Writable stream is closed or errored.` aborting renders on Linux. `execProcess` now awaits and swallows the rejection from `process.stdin.close()` when the child closes its stdin first. The captured stderr is now also surfaced when `typst-gather analyze` falls back to staging all packages, so failures are diagnosable without bypassing `quarto`.
Expand Down
15 changes: 14 additions & 1 deletion src/execute/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
partitionYamlFrontMatter,
readYamlFromMarkdown,
} from "../core/yaml.ts";
import { dirAndStem } from "../core/path.ts";
import { dirAndStem, normalizePath } from "../core/path.ts";

import { metadataAsFormat } from "../config/metadata.ts";
import { kBaseFormat, kEngine } from "../config/constants.ts";
Expand Down Expand Up @@ -355,6 +355,19 @@ export async function fileExecutionEngineAndTarget(
flags: RenderFlags | undefined,
project: ProjectContext,
): Promise<{ engine: ExecutionEngineInstance; target: ExecutionTarget }> {
// Establish an "always absolute" contract for the file path at this single
// convergence point for all callers (preview format-resolution, render, and
// project render). fileInformationCache keys are normalized to absolute
// (FileInformationCacheMap), so a caller passing a cwd-relative path — e.g.
// preview seeding the cache with the bare filename RStudio's Render button
// uses from a subdirectory — would otherwise store a target whose
// source/input stay relative under an absolute key, a stale value that a
// later absolute-path lookup reuses. Normalizing here keeps target.source and
// target.input consistent with the key and with project renders (which
// already pass absolute paths). See #14151 (the contract), #12401
// (QUARTO_DOCUMENT_PATH / cwd inconsistency), #14150 (doubled subdirectory),
// and #14683 (knitr abs_path failure previewing from a subdirectory).
file = normalizePath(file);
const cached = ensureFileInformationCache(project, file);
if (cached && cached.engine && cached.target) {
return { engine: cached.engine, target: cached.target };
Expand Down
7 changes: 7 additions & 0 deletions tests/docs/manual/preview/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,13 @@ After every change to preview URL or handler logic, verify that single-file prev
- **Expected:** HTTP 200. Browse URL may include path for non-index files in project context.
- **Catches:** `isSingleFile` guard accidentally excluding real project files from path computation

#### T33: knitr doc in a subdirectory — preview from that subdirectory (#14683)

- **Setup:** Fixture `knitr-subdir-14683/` — a website project with a knitr doc at `labs/doc.qmd` (an `{r}` chunk plus a `QUARTO_DOCUMENT_PATH` echo). Requires R + `rmarkdown`/`knitr`.
- **Steps:** From `knitr-subdir-14683/labs/` (the doc's own directory), run `quarto preview doc.qmd --to html --no-watch-inputs --no-browser --port XXXX` — the shape RStudio's Render button uses (bare filename, cwd = the doc's subdirectory).
- **Expected:** Renders without `Error in rmarkdown:::abs_path(input)`; `_site/labs/doc.html` is produced showing `[1] 2` (the R chunk executed). The echoed `QUARTO_DOCUMENT_PATH` is the doc's absolute subdirectory (ends in `labs`), not `.` / the project root (#12401).
- **Catches:** `fileExecutionEngineAndTarget` not normalizing the input to absolute — a cwd-relative `target.input`/`source` in the shared preview cache makes the knitr R subprocess (cwd = project dir) fail `abs_path` on the subdirectory-relative filename.

## Test Matrix: Format Change Detection (#14533)

After every change to `previewRenderRequestIsCompatible` or the
Expand Down
4 changes: 4 additions & 0 deletions tests/docs/manual/preview/knitr-subdir-14683/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/.quarto/
/_site/
**/*_files/
**/*.quarto_ipynb
5 changes: 5 additions & 0 deletions tests/docs/manual/preview/knitr-subdir-14683/_quarto.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
project:
type: website

website:
title: "Knitr Subdir Preview (#14683)"
6 changes: 6 additions & 0 deletions tests/docs/manual/preview/knitr-subdir-14683/index.qmd
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
title: Home
---

Home page. The regression fixture is `labs/doc.qmd` — preview it from the
`labs/` directory with a bare filename (see T33).
20 changes: 20 additions & 0 deletions tests/docs/manual/preview/knitr-subdir-14683/labs/doc.qmd
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
title: Claude Test
engine: knitr
---

A knitr document in a project subdirectory. Previewing it from this `labs/`
directory with a bare filename (`quarto preview doc.qmd`) is the shape
RStudio's Render button uses, and is what regressed in #14683.

```{r}
1 + 1
```

The rendered output must show `[1] 2` above (the R engine executed), and
`QUARTO_DOCUMENT_PATH` below must be this file's absolute directory (ending in
`labs`), not `.` or the project root (#12401).

```{r}
cat("QUARTO_DOCUMENT_PATH:", Sys.getenv("QUARTO_DOCUMENT_PATH"))
```
Loading
Loading