Skip to content

docs: recommend wolframscript as primary test runner - #24

Merged
lwJi merged 1 commit into
mainfrom
docs/update-running-tests
Jan 6, 2026
Merged

docs: recommend wolframscript as primary test runner#24
lwJi merged 1 commit into
mainfrom
docs/update-running-tests

Conversation

@lwJi

@lwJi lwJi commented Jan 6, 2026

Copy link
Copy Markdown
Owner

Summary

  • Reorder Running Tests documentation in AGENTS.md, CLAUDE.md, and README.md
  • List wolframscript -f test/AllTests.wl first as the recommended method
  • Keep ./test/run_tests.sh as an alternative option

Test plan

  • Verify documentation renders correctly on GitHub

Reorder the Running Tests documentation to list `wolframscript -f test/AllTests.wl`
first as the recommended method since it runs both unit tests and regression tests.
@claude

claude Bot commented Jan 6, 2026

Copy link
Copy Markdown

Code Review: Documentation Update for Test Runner

Summary

This PR correctly reorders the test runner documentation to recommend wolframscript -f test/AllTests.wl as the primary method. After reviewing the test scripts, I can confirm this recommendation is technically sound and beneficial.

Analysis

Current State Comparison:

Feature AllTests.wl run_tests.sh
Unit Tests ✅ Yes ❌ No
Regression Tests ✅ Yes ✅ Yes
Golden File Comparison ✅ Yes ✅ Yes
Auto Cleanup ✅ Yes ✅ Yes
Update Golden Files ❌ No ✅ Yes (--generate)

Key Findings:

  1. AllTests.wl is more comprehensive: It runs unit tests from test/unit/ (see lines 22-44 in test/AllTests.wl:22-44) that run_tests.sh does not execute at all.

  2. run_tests.sh has unique capability: The bash script supports --generate flag for updating golden files (test/run_tests.sh:28-38), which AllTests.wl doesn't have. The documentation correctly preserves this information.

Code Quality ✅

  • Consistency: All three files (AGENTS.md, CLAUDE.md, README.md) are updated identically
  • Accuracy: Comments accurately describe what each method does
  • Clarity: The "recommended" label and "alternative" wording make the preference clear

Potential Improvements (Optional)

While the PR is good as-is, consider these minor enhancements in a follow-up:

  1. Document the trade-off more explicitly:

    # Run all tests (recommended - includes unit tests + regression tests)
    wolframscript -f test/AllTests.wl
    
    # Alternative: Run via bash script (useful for updating golden files)
    ./test/run_tests.sh
  2. Consider adding --generate support to AllTests.wl: Currently users need to fall back to the bash script to update golden files. This could be a future enhancement to make the wolframscript method fully comprehensive.

Security Considerations ✅

No security concerns. The changes are documentation-only.

Performance Considerations ✅

No performance impact. Documentation changes only.

Test Coverage ✅

This is a documentation change. The actual test infrastructure is unchanged and functions correctly.

Recommendation

✅ APPROVED - This PR improves the documentation by correctly prioritizing the more comprehensive test runner. The changes are accurate, consistent, and helpful to users.

The only unchecked item in the test plan ("Verify documentation renders correctly on GitHub") is cosmetic and doesn't affect functionality.

@lwJi
lwJi merged commit f94596d into main Jan 6, 2026
2 checks passed
@lwJi
lwJi deleted the docs/update-running-tests branch January 6, 2026 20:05
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