Skip to content

test: harden test262 tooling regression coverage#401

Merged
dowdiness merged 3 commits into
mainfrom
chore/test262-tooling-cleanups
Jun 18, 2026
Merged

test: harden test262 tooling regression coverage#401
dowdiness merged 3 commits into
mainfrom
chore/test262-tooling-cleanups

Conversation

@dowdiness

@dowdiness dowdiness commented Jun 18, 2026

Copy link
Copy Markdown
Owner

Summary

  • Added regression coverage for Atomics timeout harness behavior under atomicsTimeoutProfile=short.
  • Added a regression test for build_test_tasks default-mode fallback when metadata read/parsing fails.
  • Migrated deprecated constructs in tooling tests:
    • try?try/catch fallback in tooling/test262_runner/task_selection.mbt
    • String.substring(...) → view slicing in tooling/subprocess_helpers/report_test262.mbt

Validation

  • moon check --target native tooling/test262_runner tooling/subprocess_helpers
  • moon test --target native tooling/test262_runner tooling/subprocess_helpers
  • moon test --target native (full repo): all tests pass

Summary by CodeRabbit

  • Tests

    • Added test cases for error handling in test task building and atomics timeout normalization to improve test coverage.
  • Refactor

    • Optimized internal shard-filename parsing logic to use efficient slice syntax.
    • Enhanced error handling in test task selection to gracefully fall back to defaults when metadata reading fails.

@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7e189587-b09f-4791-a53a-a35e05240fb8

📥 Commits

Reviewing files that changed from the base of the PR and between ef35054 and f134106.

📒 Files selected for processing (3)
  • tooling/subprocess_helpers/report_test262.mbt
  • tooling/test262_runner/runner_wbtest.mbt
  • tooling/test262_runner/task_selection.mbt

📝 Walkthrough

Walkthrough

Refactors build_test_tasks error handling from try?/match to an explicit try/catch block that falls back to default_modes_for_run_mode. Switches shard string extraction in report_test262.mbt from substring calls to slice syntax. Adds two new test cases: one for the build_test_tasks fallback behavior and one for atomics short-timeout preamble normalization.

Changes

test262 runner and subprocess helper improvements

Layer / File(s) Summary
build_test_tasks error handling and fallback test
tooling/test262_runner/task_selection.mbt, tooling/test262_runner/runner_wbtest.mbt
build_test_tasks replaces try?/match with try { ... } catch { ... } that falls back to default_modes_for_run_mode(run_mode) on any error. A new test passes missing/invalid paths alongside a valid one and asserts default non-strict and strict entries are produced for all inputs.
Atomics short-timeout preamble normalization test
tooling/test262_runner/runner_wbtest.mbt
Adds a test constructing HarnessOptions with atomics_timeout_profile=ATOMICS_TIMEOUT_PROFILE_SHORT and asserting the generated preamble includes NaN-handling/normalization code and bounded atomics-wait wrapper calls, while excluding a specific negative-type guard string.
Shard filename parsing: substring → slice syntax
tooling/subprocess_helpers/report_test262.mbt
parse_shard_of_total extracts the left/right numeric parts of NofM using inner[:of_pos] and inner[of_pos + 2:]; validate_shard_completeness extracts the shard inner portion using slice indices derived from prefix/suffix lengths.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • dowdiness/js_engine#297: Introduced shard completeness validation and NofM filename parsing logic in report_test262.mbt that this PR directly refactors.
  • dowdiness/js_engine#299: Modifies parse_shard_of_total and validate_shard_completeness in the same file and functions updated in this PR.

Poem

🐇 A slice is cleaner than a substr call,
A catch replaces match once and for all.
I hop through the harness with short-timeout care,
NaN-handling tested, with fallbacks to spare.
The shard finds its number with elegant grace —
One tiny refactor, a tidier place!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'test: harden test262 tooling regression coverage' clearly describes the main focus of the changeset: adding regression test coverage to the test262 tooling, which is the primary objective across all three modified files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/test262-tooling-cleanups

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@dowdiness dowdiness merged commit 3f49b1e into main Jun 18, 2026
12 checks passed
@dowdiness dowdiness deleted the chore/test262-tooling-cleanups branch June 18, 2026 15:12
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.

1 participant