Skip to content

super_errors_multi: multi-file fixture harness for cross-module errors#8433

Merged
JonoPrest merged 16 commits into
rescript-lang:masterfrom
JonoPrest:jono/expand-coverage-3
May 26, 2026
Merged

super_errors_multi: multi-file fixture harness for cross-module errors#8433
JonoPrest merged 16 commits into
rescript-lang:masterfrom
JonoPrest:jono/expand-coverage-3

Conversation

@JonoPrest
Copy link
Copy Markdown
Contributor

Follow-up to #8432 (jono/expand-coverage-2). Adds a new test directory
tests/build_tests/super_errors_multi/ that mirrors super_errors/ but for
errors/warnings that only surface across compilation-unit boundaries.

Why a new harness

The existing super_errors/ runner only feeds one .res file to bsc per
fixture. That covers everything reachable from a single source file but
leaves a class of compiler errors untested:

  • Signature mismatch between .resi and .res (Includemod through
    compunit).
  • Cross-module value / type / record / variant disambiguation.
  • Cross-module abstract type construction, private-type guards.
  • Cross-module deprecation warnings and shadowing warnings.
  • Interface_not_compiled (missing .cmi when -bs-read-cmi is set).

This PR adds a parallel directory with its own runner that compiles each
fixture's files via bsc directly, ordering .resi ahead of .res per
module and tagging output by source file.

Harness design

  • Fixture format: one subdirectory per fixture under fixtures/.
    Files inside are compiled alphabetically (with .resi ahead of the
    matching .res). Fixtures needing a specific cross-module order can
    prefix filenames with a numeric label (e.g. 01_Foo.res).
  • bsc flags: -w +A -bs-jsx 4 -color always -bs-cmi-only, plus
    -bs-read-cmi per file when a corresponding .resi exists. -bs-cmi-only
    stops the pipeline after typechecking, avoiding the "Js_not_found" failure
    bsc otherwise raises when trying to resolve sibling .js for cross-module
    imports.
  • Snapshots: one expected/<FixtureName>.expected per fixture with all
    files' stderr concatenated, tagged ===== Foo.res =====.
  • Runner parallelism: worker pool over fixtures, mirroring the
    single-file runner (super_errors: run fixtures in parallel #8430).
  • Artifact hygiene: .cmi/.cmj/.cmt/.cmti/.mjs/.js are cleaned per
    fixture before each run and globally gitignored.

Format / format-check scripts are extended to skip
fixtures/Iface_not_compiled/ since that fixture intentionally contains a
.resi with a parse error.

What's covered (multi-file fixtures by category)

Includemod via .resi / .res mismatch (through compunit):

  • Iface_missing_value (Missing_field)
  • Iface_value_descriptions
  • Iface_type_decl_record / variant
  • Iface_extension_constructors
  • Iface_module_types
  • Iface_modtype_infos
  • Iface_not_compiled (Interface_not_compiled)
  • Iface_privacy_mismatch (Includecore Privacy)
  • Iface_field_mutable_mismatch (Includecore Field_mutable)
  • Iface_field_optional_mismatch (Includecore Field_optional)
  • Iface_unboxed_variant_mismatch (Includecore Unboxed_representation)
  • Iface_tag_name_mismatch (Includecore Tag_name)
  • Iface_variant_representation_mismatch (Includecore Variant_representation)

Cross-module type / record / constructor disambiguation:

  • Smoke_cross_module_type_clash
  • Smoke_missing_field
  • Smoke_unbound_module_reference
  • Cross_abstract_type_construction
  • Cross_qualified_constructor_mismatch (Name_type_mismatch via cross-module
    constructor)
  • Cross_module_record_disambiguation
  • Cross_record_extra_field (Wrong_name via cross-module label)
  • Cross_chain_of_aliases (module B = A.Inner, C types against
    B.Reexport.t)
  • Cross_dependent_clash
  • Cross_polyvariant_constraint
  • Cross_module_alias_dot_access (module S = Lib.Settings; S.host)
  • Cross_inline_record_constructor
  • Cross_variant_spread (type b = ...A.t | C overlap)
  • Cross_hidden_through_interface (record access on type abstracted by .resi)
  • Cross_uncurried_arity (function arity mismatch via cross-module alias)
  • Cross_module_type_only_no_impl (consumer typechecks against .resi alone)

Async / FFI / patterns / GADTs:

  • Cross_async_promise_mismatch
  • Cross_optional_label_omitted
  • Cross_dict_pattern
  • Cross_external_argument_clash
  • Cross_private_constructor_pattern
  • Cross_exception_arity_mismatch
  • Cross_gadt_pattern (Unqualified_gadt_pattern — was reachable only here,
    not from a single file)
  • Modules_not_allowed_toplevel (toplevel let module(M) = … pattern)
  • Unexpected_existential_in_let (destructuring GADT constructor with existential
    in toplevel let)

Warnings across modules:

  • Cross_unused_open (warning 33)
  • Cross_deprecated_value (warning 3 via cross-module @deprecated @val
    external)
  • Cross_deprecated_type (warning 3 on @deprecated type used cross-module)
  • Cross_open_shadows_identifier (warning 44)
  • Cross_module_open_shadow_label_constructor (warnings 45 + 41)

Limitations

  • The harness uses raw bsc rather than rescript build, so fixtures can't
    rely on rescript.json, namespacing, or third-party packages. JSX fixtures
    that import React were ruled out for this reason.
  • Cannot_scrape_alias (typetexp / typemod) requires a broken alias chain
    in .cmi data that a hand-written fixture can't produce.
  • Scoping_pack, With_makes_applicative_functor_ill_typed,
    With_changes_module_alias, Cannot_eliminate_dependency all need
    applicative-functor or destructive-substitution constructions that didn't
    fire from any fixture I tried. Likely dead in current ReScript syntax;
    flagged for a separate audit.
  • Includemod.Unbound_module_path / Unbound_modtype_path /
    Invalid_module_alias require signature comparisons whose triggers couldn't
    be reproduced from the fixture format. Also flagged.

Coverage impact

Measured locally with make coverage (bisect_ppx point coverage). Baseline
is upstream/master (commit at the time of this PR).

Metric Upstream/master This branch Δ
Overall 52.11% (38049 / 73012) 52.63% (38428 / 73012) +0.52 pp / +379 points

Per-file moves on the modules this branch targets:

File Upstream This branch Δ pp Points
compiler/ml/includecore.ml 54.70% 59.54% +4.84 +17
compiler/ml/includemod.ml 63.17% 67.99% +4.82 +17
compiler/ml/typemod.ml 58.58% 60.17% +1.59 +15
compiler/ml/typedecl.ml 68.62% 70.08% +1.46 +19
compiler/ml/translmod.ml 71.56% 72.94% +1.38 +3
compiler/ml/typecore.ml 75.53% 76.81% +1.28 +37
compiler/ml/typetexp.ml 74.38% 75.62% +1.24 +7
compiler/ml/env.ml 68.16% 69.24% +1.08 +13
compiler/ml/translcore.ml 71.45% 71.82% +0.37 +2

The two ~+5 pp jumps on includecore.ml and includemod.ml are the
headline — those are precisely the modules that the .resi/.res mismatch
fixtures (Iface_*) exercise via the Includemod.compunit entry point,
which has no equivalent in single-file work. Privacy, mutable-field,
optional-field, unboxed-rep, tag-name, and variant-representation paths in
includecore are now hit. The +37 absolute points on typecore.ml come
from cross-module GADT disambiguation (Unqualified_gadt_pattern),
Modules_not_allowed, Unexpected_existential, Cannot_infer_signature,
and Undefined_method.

Modules where the single-file suite already topped out the reachable paths
(rec_check.ml, transl_recmodule.ml, bs_syntaxerr.ml, ast_attributes.ml,
ast_external_process.ml, cmi_format.ml) show 0 movement — multi-file
doesn't unlock anything new in those.

JonoPrest added a commit to JonoPrest/rescript-compiler that referenced this pull request May 20, 2026
@JonoPrest JonoPrest marked this pull request as ready for review May 20, 2026 16:27
@JonoPrest
Copy link
Copy Markdown
Contributor Author

@codex review

@JonoPrest JonoPrest requested a review from cknitt May 20, 2026 16:31
@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Bravo.

ℹ️ 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".

@codecov
Copy link
Copy Markdown

codecov Bot commented May 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.10%. Comparing base (4f27edd) to head (a7e68e5).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8433      +/-   ##
==========================================
+ Coverage   59.90%   60.10%   +0.19%     
==========================================
  Files         373      373              
  Lines       54231    54231              
==========================================
+ Hits        32489    32593     +104     
+ Misses      21742    21638     -104     

see 19 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
Copy link
Copy Markdown

pkg-pr-new Bot commented May 20, 2026

Open in StackBlitz

rescript

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

@rescript/darwin-arm64

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

@rescript/darwin-x64

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

@rescript/linux-arm64

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

@rescript/linux-x64

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

@rescript/runtime

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

@rescript/win32-x64

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

commit: a7e68e5

Copy link
Copy Markdown
Member

@cknitt cknitt left a comment

Choose a reason for hiding this comment

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

Nice work! 🎉

@JonoPrest
Copy link
Copy Markdown
Contributor Author

JonoPrest commented May 26, 2026

Nice work! 🎉

Thanks, I have 2 PRs ready after this is merged but I don't know how best to stack/stage them for review without making a huge PR.

Next 2 are:

  • last sweep of reachable variants with this single and multi file harness
  • removing dead/unreachable errors

I will then do a once over to try and improve error messages that aren't so good right now (which these tests have surfaced)

And that should conclude this work on error coverage. We can squeeze out more sub cases but I don't think its worth much.

JonoPrest added 16 commits May 26, 2026 16:27
…ptional, Unboxed_representation, Tag_name mismatches
…ssing

`find_in_path_uncap` resolves `.cmi` lookups case-insensitively, so the
filename printed in error messages picks up whichever case the host
filesystem returns: lowercase on macOS APFS/HFS+, capitalised on Linux
ext4. Snapshots captured locally on macOS therefore mismatch on Linux
CI (`foo.cmi` vs `Foo.cmi`).

Normalise every `.cmi` basename to lowercase in postProcessErrorOutput
so snapshots are platform-independent. The same function runs on the
expected file too, so existing snapshots keep matching.
@JonoPrest JonoPrest force-pushed the jono/expand-coverage-3 branch from dcd0c8e to a7e68e5 Compare May 26, 2026 14:29
@JonoPrest JonoPrest enabled auto-merge (squash) May 26, 2026 14:31
@JonoPrest JonoPrest merged commit 0a29fc8 into rescript-lang:master May 26, 2026
29 checks passed
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