Skip to content

Stop tools tests on unexpected failures - #8545

Merged
cknitt merged 1 commit into
rescript-lang:masterfrom
cknitt:codex/handle-expected-tools-test-failures
Aug 13, 2026
Merged

Stop tools tests on unexpected failures#8545
cknitt merged 1 commit into
rescript-lang:masterfrom
cknitt:codex/handle-expected-tools-test-failures

Conversation

@cknitt

@cknitt cknitt commented Aug 13, 2026

Copy link
Copy Markdown
Member

Summary

  • make the tools test script stop on unexpected command failures
  • handle the intentionally invalid docstring fixture as an expected failure for both format-codeblocks and extract-codeblocks
  • allow the script to run when RUNNER_OS is unset

Why

The invalid docstring fixture correctly makes the tools commands exit with a non-zero status. The test script previously ignored those statuses and continued, so make test-all printed a prominent syntax error while still treating the tools suite as successful. The same behavior could also hide genuinely unexpected command failures.

The negative fixture is now asserted explicitly: the suite fails if either command unexpectedly succeeds, while all other failures stop the script immediately.

Testing

  • verified the formatter and extractor fail for FormatDocstringsTestError.res and produce the checked-in expected output
  • make -C tests/tools_tests test
  • bash -n tests/tools_tests/test.sh

Signed-off-by: Christoph Knittel <ck@cca.io>
@cknitt cknitt changed the title Handle expected failures in tools tests Stop tools tests on unexpected failures Aug 13, 2026
@cknitt
cknitt requested a review from fhammerschmidt August 13, 2026 11:41
@cknitt
cknitt marked this pull request as ready for review August 13, 2026 11:44
Comment thread tests/tools_tests/test.sh
../../_build/install/default/bin/rescript-tools doc $file > $output
# # CI. We use LF, and the CI OCaml fork prints CRLF. Convert.
if [ "$RUNNER_OS" == "Windows" ]; then
if [ "${RUNNER_OS:-}" == "Windows" ]; then

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:-}

@pkg-pr-new

pkg-pr-new Bot commented Aug 13, 2026

Copy link
Copy Markdown

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript@8545

@rescript/darwin-arm64

npm i https://pkg.pr.new/@rescript/darwin-arm64@8545

@rescript/darwin-x64

npm i https://pkg.pr.new/@rescript/darwin-x64@8545

@rescript/linux-arm64

npm i https://pkg.pr.new/@rescript/linux-arm64@8545

@rescript/linux-x64

npm i https://pkg.pr.new/@rescript/linux-x64@8545

@rescript/runtime

npm i https://pkg.pr.new/@rescript/runtime@8545

@rescript/win32-x64

npm i https://pkg.pr.new/@rescript/win32-x64@8545

commit: 67eee62

@cknitt
cknitt merged commit 0c1bc3d into rescript-lang:master Aug 13, 2026
29 checks passed
@cknitt
cknitt deleted the codex/handle-expected-tools-test-failures branch August 13, 2026 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants