Fix run-tests.sh silently truncating recursive ** glob buckets#14700
Merged
Conversation
Bash only expands ** recursively when globstar is enabled, and run-tests.sh passed its resolved TESTS_TO_RUN unquoted to the final deno invocation, letting bash glob-expand it with globstar off. A bucket like qmd-files/**/*.qmd matched only files exactly one directory level deep, silently dropping anything nested further - which is why the ff-matrix CI job ran 10 tests on ubuntu-latest against 271 on windows-latest for the same bucket (run-tests.ps1 never shell-expands the pattern, deferring to Deno's own recursive glob support in smoke-all.test.ts). A prior attempt to fix this (9d718d1) added `shopt -s globstar` to the CI wrapper around the run-tests.sh call, but that only affects the wrapper step's own shell - run-tests.sh runs as a separate bash process with its own default shell-option state, so the setting never reached the actual expansion. Convert TESTS_TO_RUN and SMOKE_ALL_FILES to bash arrays and quote them at the deno invocation so bash never expands the pattern itself; a literal glob string now reaches smoke-all.test.ts's own expandGlobSync, matching the working Windows code path instead of relying on a second, divergent glob engine. Also drops the now-dead `shopt -s globstar` from the CI wrapper. Adds tests/run-tests-recursive-glob.test.sh, a standalone regression test that runs the real run-tests.sh against a nested qmd fixture with a recorder-stub deno binary, confirming a depth-2 file survives a ** bucket.
Collaborator
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Running the full qmd-files tree locally (as now happens in CI after the run-tests.sh recursive-glob fix) leaves keep-typ and ipynb-cache byproducts untracked and unignored.
Georgia is a Windows/macOS-bundled font absent on Ubuntu CI runners. Typst CSS font-family filtering (#12556) silently drops unavailable fonts, so the same document rendered `("Georgia", "serif")` on Windows/macOS but `("serif",)` on Ubuntu. This went unnoticed because the fixture never actually ran in CI until the run-tests.sh recursive glob fix. Libertinus Serif ships embedded in the Typst binary, so it is present on every host regardless of installed system fonts.
…v token The multi-file loop in run-tests.sh iterated `"$*"` (all positional args joined into one string) instead of `"$@"`, so passing several .test.ts or .qmd files at once - documented in tests/README.md - only ever produced a single glued token. Quoting TESTS_TO_RUN as an array (acc33e7) removed the accidental unquoted word-splitting that used to reconstruct the separate args, exposing the bug as a real regression.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On Linux and macOS CI, a
**glob bucket passed torun-tests.sh(e.g.qmd-files/**/*.qmdfrom the feature-format matrix job) only picks up files exactly one directory level deep, silently dropping anything nested further. The ff-matrix job ran 10 tests onubuntu-latestagainst 271 onwindows-latestfor the identical bucket.Root Cause
run-tests.shbuilds its resolved file list and passes it unquoted to the finaldeno testinvocation. Bash then glob-expands that list itself, and withoutglobstarenabled,**behaves like a single*, matching only one directory level.A prior fix added
shopt -s globstarto the CI wrapper around therun-tests.shcall (9d718d1). That only affects the wrapper step's own shell, though -run-tests.shruns as a separate bash process with its own default shell-option state, so the setting never reached the actual expansion and the fix was a no-op.Fix
Convert the resolved file list to a bash array and quote it at the
denoinvocation, so bash never touches the pattern. A literal, unexpanded glob string now reachessmoke-all.test.ts, which already expands it itself viaexpandGlobSync- the same mechanism Windows already relies on, since PowerShell never shell-expands the pattern either. Also removes the now-deadshopt -s globstarfrom the CI wrapper.Adds a standalone regression test (
tests/run-tests-recursive-glob.test.sh) that runs the realrun-tests.shagainst a nested fixture with a stubbeddenobinary, confirming a file two directories deep survives a**bucket.