Skip to content

Commit 7a505ec

Browse files
authored
Merge pull request #13963 from quarto-dev/ojs-r-number
`ojs_define` should not round numeric
1 parent 6c8a9b1 commit 7a505ec

11 files changed

Lines changed: 227 additions & 5 deletions

File tree

llm-docs/testing-patterns.md

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,45 @@ Quarto uses Deno for testing with custom verification helpers located in:
1111

1212
## Common Test Patterns
1313

14+
### Simple Render Tests
15+
16+
For testing single document rendering with automatic cleanup:
17+
18+
```typescript
19+
import { docs } from "../../utils.ts";
20+
import { testRender } from "./render.ts";
21+
22+
// Simplest form - just render and verify output created
23+
testRender(docs("test-plain.md"), "html", false);
24+
```
25+
26+
**With additional verifiers:**
27+
28+
```typescript
29+
import { docs, outputForInput } from "../../utils.ts";
30+
import { testRender } from "./render.ts";
31+
import { ensureHtmlElements } from "../../verify.ts";
32+
33+
const input = docs("minimal.qmd");
34+
const output = outputForInput(input, "html");
35+
36+
testRender(input, "html", true, [
37+
ensureHtmlElements(output.outputPath, [], [
38+
"script#quarto-html-after-body",
39+
]),
40+
]);
41+
```
42+
43+
**Key points:**
44+
- `testRender()` automatically handles output verification and cleanup
45+
- Respects `QUARTO_TEST_KEEP_OUTPUTS` env var for debugging
46+
- Set `noSupporting` parameter based on expected output:
47+
- `true` - For truly self-contained HTML (no `_files/` directory, inline everything)
48+
- `false` - For HTML with supporting files directory (OJS runtime, widget dependencies, plots, etc.)
49+
- Most HTML outputs should use `false` (only use `true` for formats like `html` with `self-contained: true`)
50+
- Pass additional verifiers in the array parameter (optional)
51+
- Cleanup happens automatically via `cleanoutput()` in teardown
52+
1453
### Project Rendering Tests
1554

1655
For testing project rendering (especially website projects):
@@ -226,6 +265,11 @@ testQuartoCmd(...);
226265

227266
## Examples from Codebase
228267

268+
### Simple Render Test
269+
See `tests/smoke/render/render-plain.test.ts` for the simplest render tests (no additional verifiers).
270+
271+
See `tests/smoke/render/render-minimal.test.ts` for render test with custom HTML element verification.
272+
229273
### Project Ignore Test
230274
See `tests/smoke/project/project-ignore-dirs.test.ts` for testing directory exclusion patterns.
231275

@@ -235,6 +279,56 @@ See `tests/smoke/project/project-website.test.ts` for website project rendering
235279
### Template Usage Test
236280
See `tests/smoke/use/template.test.ts` for extension template patterns.
237281

282+
## Engine-Specific Test Considerations
283+
284+
### Shared Test Environments (Critical for quarto-cli Testing)
285+
286+
**Quarto-cli test infrastructure uses a SINGLE managed environment for all tests:**
287+
288+
- **Julia**: `tests/Project.toml` + `tests/Manifest.toml`
289+
- **Python**: `tests/.venv/` (managed by uv/pyproject.toml)
290+
- **R**: `tests/renv/` + `tests/renv.lock`
291+
292+
The `configure-test-env` scripts ONLY manage these main environments. CI builds depend on this structure.
293+
294+
**Do NOT create language environment files in test subdirectories:**
295+
296+
```
297+
tests/docs/my-test/
298+
├── Project.toml # ❌ WRONG - breaks test infrastructure
299+
├── .venv/ # ❌ WRONG - breaks test infrastructure
300+
├── renv.lock # ❌ WRONG - breaks test infrastructure
301+
└── test.qmd
302+
```
303+
304+
**Why this fails:**
305+
- Julia searches UP for `Project.toml` and uses the first one found
306+
- Python/R will use local environments if present
307+
- CI scripts won't configure these local environments
308+
- Tests will fail in CI even if they work locally
309+
310+
**Adding new package dependencies:**
311+
312+
For ANY engine (Julia, Python, R), add dependencies to the main `tests/` environment:
313+
314+
```bash
315+
# Julia: Use Pkg from tests/ directory
316+
cd tests
317+
julia --project=. -e 'using Pkg; Pkg.add("PackageName")'
318+
# Then run configure to update environment
319+
./configure-test-env.sh # or .ps1 on Windows
320+
321+
# Python: Use uv from tests/ directory
322+
cd tests
323+
uv add packagename
324+
325+
# R: Edit tests/DESCRIPTION, then
326+
cd tests
327+
Rscript -e "renv::install(); renv::snapshot()"
328+
```
329+
330+
**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.
331+
238332
## Best Practices
239333

240334
1. **Always clean up**: Use teardown to remove generated files

news/changelog-1.9.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,10 @@ All changes included in 1.9:
132132

133133
- ([#13748](https://github.com/quarto-dev/quarto-cli/pull/13748)): Fix stdin encoding to UTF-8 on Windows to correctly handle JSON in documents containing non-ASCII characters.
134134

135+
### `knitr`
136+
137+
- ([#13958](https://github.com/quarto-dev/quarto-cli/issues/13958)): Use max precision for `ojs_define` number like this is the case for `jupyter` or `julia` engine.
138+
135139
## Other fixes and improvements
136140

137141
- ([#8730](https://github.com/quarto-dev/quarto-cli/issues/8730)): Detect x64 R crashes on Windows ARM and provide helpful error message directing users to install native ARM64 R instead of showing generic "check your R installation" error.

src/resources/rmd/ojs_static.R

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ ojs_define <- function(...) {
4545
dataframe = "columns",
4646
null = "null",
4747
na = "null",
48-
auto_unbox = TRUE
48+
auto_unbox = TRUE,
49+
digits = NA
4950
)
5051
script_string <- c(
5152
"<script type=\"ojs-define\">",

tests/Manifest.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
julia_version = "1.11.7"
44
manifest_format = "2.0"
5-
project_hash = "0af1d64f93f9c216a25d220e89fda045c4edc0aa"
5+
project_hash = "395f6c17c85163e28668018017fa1e17f23a2784"
66

77
[[deps.AliasTables]]
88
deps = ["PtrArrays", "Random"]

tests/Project.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
[deps]
22
IJulia = "7073ff75-c697-5162-941a-fcdaad2a7d2a"
3+
JSON = "682c06a0-de6a-54ab-a142-c8b1cf79cde6"
34
Plots = "91a5bcdd-55d7-5caf-9e0b-520d859cae80"

tests/README.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,20 @@ Julia uses built-in package manager [**Pkg.jl**](https://pkgdocs.julialang.org/v
118118

119119
`Project.toml` contains our direct dependency and `Manifest.toml` is the lock file that will be created (`Pkg.resolve()`).
120120

121+
**Important:** All test dependencies must be in the main `tests/` environment. Julia searches UP the directory tree for `Project.toml` starting from the document being rendered.
122+
123+
**Adding a new package dependency:**
124+
125+
```bash
126+
cd tests
127+
julia --project=. -e 'using Pkg; Pkg.add("PackageName")'
128+
./configure-test-env.sh # or .ps1 on Windows
129+
```
130+
131+
**Do NOT create** local `Project.toml` files in test subdirectories (e.g., `tests/docs/*/Project.toml`). Julia will use that environment instead of the main `tests/` environment. The `configure-test-env` scripts only manage the main environment, so tests with local environments will fail in CI even if they work locally.
132+
133+
**Note:** This applies to ALL engines (Julia, Python, R). Python and R will also use local `.venv/` or `renv.lock` if present. The quarto-cli test infrastructure uses a single managed environment per language at `tests/`, and CI only configures these main environments.
134+
121135
See [documentation](https://pkgdocs.julialang.org/v1/managing-packages/) on how to add, remove, update if you need to tweak the Julia environment.
122136

123137
### How to run tests locally ?
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
title: "ojs_define numeric precision test"
3+
format: html
4+
engine: julia
5+
execute:
6+
daemon: false
7+
---
8+
9+
```{julia}
10+
using JSON
11+
ojs_define(num=0.00008604168504168504)
12+
```
13+
14+
```{ojs}
15+
num
16+
```
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
title: "ojs_define numeric precision test"
3+
format: html
4+
engine: jupyter
5+
execute:
6+
daemon: false
7+
---
8+
9+
```{python}
10+
ojs_define(num = 0.00008604168504168504)
11+
```
12+
13+
```{ojs}
14+
num
15+
```
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
title: "ojs_define numeric precision test"
3+
format: html
4+
engine: knitr
5+
---
6+
7+
```{r}
8+
ojs_define(num = 0.00008604168504168504)
9+
```
10+
11+
```{ojs}
12+
num
13+
```
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import { docs, outputForInput } from "../../utils.ts";
2+
import { assert } from "testing/asserts";
3+
import { testRender } from "../render/render.ts";
4+
import { verifyOjsDefine } from "../../verify.ts";
5+
6+
// Test for #13958: ojs_define() rounds numbers incorrectly
7+
// Test across all three execution engines: knitr, jupyter, julia
8+
const engines = ["knitr", "jupyter", "julia"];
9+
10+
for (const engine of engines) {
11+
const input = docs(`ojs/ojs-define-numeric-precision/with-${engine}.qmd`);
12+
const output = outputForInput(input, "html");
13+
14+
testRender(
15+
input,
16+
"html",
17+
false,
18+
[
19+
verifyOjsDefine(async (contents) => {
20+
const numEntry = contents.find((item) => item.name === "num");
21+
assert(numEntry, "Should find 'num' variable in ojs-define data");
22+
assert(
23+
typeof numEntry.value === "number",
24+
"ojs-define value should be a number"
25+
);
26+
27+
// Validate numeric precision (the actual bug test)
28+
const expected = 0.00008604168504168504;
29+
const tolerance = 1e-15; // Machine epsilon tolerance
30+
31+
const diff = Math.abs(numEntry.value - expected);
32+
assert(
33+
diff < tolerance,
34+
`ojs-define value should preserve full precision. Expected ${expected}, got ${numEntry.value}, diff ${diff}`
35+
);
36+
}, `ojs_define preserves full numeric precision (${engine})`)(output.outputPath),
37+
],
38+
);
39+
}

0 commit comments

Comments
 (0)