Skip to content

Remove the converted Reason project corpus from the syntax tests - #9012

Merged
cknitt merged 1 commit into
masterfrom
remove-idempotency-corpus
Oct 24, 2026
Merged

cknitt merged 1 commit into
masterfrom
remove-idempotency-corpus

Conversation

@cknitt

@cknitt cknitt commented Oct 24, 2026 •

Copy link
Copy Markdown
Member

tests/syntax_tests/data/idempotency held about 109k lines of old community projects converted from Reason (bs-css, reason-react, pupilfirst, …). Only the roundtrip run of scripts/test_syntax.sh used it, checking that printing twice gives the same output. Every syntax deprecation or removal had to rewrite it, and its history shows no printer bug it caught.

I compared bisect_ppx line coverage of compiler/syntax for the regular syntax tests against a roundtrip of the corpus. The corpus covered 25 lines nothing else did. Each one traces to a construct that now has a minimal snippet in an existing printer fixture:

  • module type path with a lowercase last segment (module Events: RGLEvents.t)
  • %raw string with backslash escapes
  • // comment between a JSX prop name and its value
  • type-constrained switch scrutinee; switch cases with array/tuple/object/record bodies
  • array set whose value is a binary expression
  • ternary in function position (x->(c ? f : g)(~a))
  • int/float JSX prop values
  • deeply nested callbacks (custom-layout threshold) and a callback-first call containing a nested callback with an if (switch …) condition
  • single char literal argument
  • poly variant type with a type, not a tag, in first position
  • braced block statement followed by a line comment
  • non-ASCII template literal inside a JSX child (exercises the scanner's unknown-character path during lookahead)

With these fixtures added, rerunning the comparison finds no line that only the corpus covers.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UoDHhLCpoQeKvfN6VbVcHv

Constructs only the corpus covered are now printer fixtures.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UoDHhLCpoQeKvfN6VbVcHv
Signed-off-by: Christoph Knittel <ck@cca.io>
@github-actions

Copy link
Copy Markdown

Changelog

No changelog entry.

Semantic Diff

1 commit · Full page

Remove the converted Reason project corpus from the syntax tests

3448ada · cknitt + Claude · Oct 24, 2026, 07:34 UTC

Constructs only the corpus covered are now printer fixtures.

@cknitt

cknitt commented Oct 24, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-24T07:37:41.661256Z 3448ada Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: 3448ada827

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@cknitt cknitt added the changelog:skip No release notes needed label Oct 24, 2026
@cknitt
cknitt marked this pull request as ready for review October 24, 2026 07:38
@cknitt
cknitt requested a review from cristianoc October 24, 2026 07:38
@cknitt
cknitt enabled auto-merge (squash) October 24, 2026 07:38
@codecov

codecov Bot commented Oct 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.31%. Comparing base (a42a937) to head (3448ada).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9012      +/-   ##
==========================================
- Coverage   81.31%   81.31%   -0.01%     
==========================================
  Files         456      456              
  Lines       63979    63979              
==========================================
- Hits        52024    52022       -2     
- Misses      11955    11957       +2     

see 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pkg-pr-new

pkg-pr-new Bot commented Oct 24, 2026

Copy link
Copy Markdown

Open in StackBlitz

rescript

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

@rescript/belt

npm i https://pkg.pr.new/@rescript/belt@9012

@rescript/darwin-arm64

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

@rescript/linux-arm64

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

@rescript/linux-x64

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

@rescript/runtime

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

@rescript/win32-x64

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

commit: 3448ada

@github-actions

Copy link
Copy Markdown

@cknitt
cknitt merged commit a5841ba into master Oct 24, 2026
26 of 27 checks passed
@cknitt
cknitt deleted the remove-idempotency-corpus branch October 24, 2026 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog:skip No release notes needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants