Skip to content

test: fix batch-2 regression tests linking prebuilt runtime archive (post-#5398)#5403

Closed
proggeramlug wants to merge 2 commits into
mainfrom
fix/functional-batch2-runtime-archive
Closed

test: fix batch-2 regression tests linking prebuilt runtime archive (post-#5398)#5403
proggeramlug wants to merge 2 commits into
mainfrom
fix/functional-batch2-runtime-archive

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #5398, which merged with a red cargo-test. The five new
crates/perry/tests/functional_batch2_regressions.rs tests set
PERRY_NO_AUTO_OPTIMIZE=1, which links the prebuilt libperry_runtime.a
instead of rebuilding a per-app runtime. Under cargo test, the staticlib
crate-type is not produced as a side effect of building the test binary
(only the rlib is), so CI failed for all five with:

Error: Could not find libperry_runtime.a.

(They pass locally only because a prior cargo build --release left the
archive on disk.)

Fix

Adopt the same convention already used by module_import_forms.rs and the
issue_5xxx no-auto-optimize tests:

  • ensure_runtime_archive() — runs cargo build -p perry-runtime once (via
    std::sync::Once) to materialize target/debug/libperry_runtime.a.
  • Pass PERRY_RUNTIME_DIR=<target/debug> to the compile so the linker resolves
    the archive directly instead of relying on ambient discovery.

No production code changes — test harness only.

Validation

cargo test -p perry --test functional_batch2_regressions5 passed.
cargo fmt -p perry --check clean.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Enhanced regression test infrastructure to properly build and locate precompiled runtime archives during testing, improving test setup and reliability.

The five batch-2 regression tests set PERRY_NO_AUTO_OPTIMIZE=1, which links
the prebuilt libperry_runtime.a instead of rebuilding a per-app runtime. Under
`cargo test` the staticlib crate-type is not produced as a side effect of
building the test binary (only the rlib is), so CI failed with
"Could not find libperry_runtime.a".

Add the ensure_runtime_archive() helper (cargo build -p perry-runtime, once)
and pass PERRY_RUNTIME_DIR=target/debug to the compile, matching the existing
convention in module_import_forms.rs and the issue_5xxx no-auto-optimize tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The regression test harness gains explicit perry-runtime static archive support. New helper functions resolve the workspace root and Cargo target debug directory (with CARGO_TARGET_DIR override), a std::sync::Once guard triggers a one-time cargo build -p perry-runtime, and compile_and_run now sets PERRY_RUNTIME_DIR for the perry compile invocation.

Changes

Perry Runtime Archive Linking in Regression Tests

Layer / File(s) Summary
Runtime archive helpers and compile_and_run wiring
crates/perry/tests/functional_batch2_regressions.rs
Adds workspace_root(), target_debug_dir() (respecting CARGO_TARGET_DIR), a Once-guarded cargo build -p perry-runtime initializer, and runtime_dir(); sets PERRY_RUNTIME_DIR in the compile_and_run helper to point at the prebuilt archive.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐇 Hippity-hop, the runtime's in place,
A Once little guard sets the build pace.
CARGO_TARGET_DIR? No worries, we check!
Link to the archive, keep tests on deck.
The warren's regression suite hops along fine~ 🎉

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: fixing batch-2 regression tests' linking issue with the prebuilt runtime archive, appropriately referencing the related PR #5398.
Description check ✅ Passed The description covers the summary, root cause, fix approach, and validation results. However, it's missing explicit test plan checkboxes and the checklist items normally required by the template.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/functional-batch2-runtime-archive

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/perry/tests/functional_batch2_regressions.rs`:
- Around line 35-40: The `target_debug_dir()` function preserves relative paths
from the `CARGO_TARGET_DIR` environment variable as relative, which causes
incorrect resolution when `perry compile` runs with a different current
directory at line 93. Convert the `CARGO_TARGET_DIR` value to an absolute path
before joining it with "debug" by using canonicalize or a similar method to
ensure the path always resolves to the correct location regardless of the
current working directory context.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 12e9f4f6-9ef5-457c-8be4-d45850a02f93

📥 Commits

Reviewing files that changed from the base of the PR and between 54ebb6c and 820c9ce.

📒 Files selected for processing (1)
  • crates/perry/tests/functional_batch2_regressions.rs

Comment on lines +35 to +40
fn target_debug_dir() -> PathBuf {
std::env::var_os("CARGO_TARGET_DIR")
.map(PathBuf::from)
.unwrap_or_else(|| workspace_root().join("target"))
.join("debug")
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Resolve CARGO_TARGET_DIR to an absolute path before exporting PERRY_RUNTIME_DIR.

On Line 36 to Line 39, a relative CARGO_TARGET_DIR is preserved as relative. On Line 93, perry compile runs with current_dir(dir) (temp fixture), so PERRY_RUNTIME_DIR=target/debug can resolve to the wrong directory and fail runtime archive lookup.

Suggested fix
 fn target_debug_dir() -> PathBuf {
     std::env::var_os("CARGO_TARGET_DIR")
         .map(PathBuf::from)
+        .map(|p| if p.is_absolute() { p } else { workspace_root().join(p) })
         .unwrap_or_else(|| workspace_root().join("target"))
         .join("debug")
 }

Also applies to: 93-93

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry/tests/functional_batch2_regressions.rs` around lines 35 - 40,
The `target_debug_dir()` function preserves relative paths from the
`CARGO_TARGET_DIR` environment variable as relative, which causes incorrect
resolution when `perry compile` runs with a different current directory at line
93. Convert the `CARGO_TARGET_DIR` value to an absolute path before joining it
with "debug" by using canonicalize or a similar method to ensure the path always
resolves to the correct location regardless of the current working directory
context.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Superseded by #5406, which fixed the same functional_batch2_regressions 'Could not find libperry_runtime.a' failure at the root (runtime #[used] keepalive anchor for js_array_numeric_value_to_raw_f64 so the linker no longer dead-strips it) plus the CI staticlib build in test.yml, rather than this test-harness workaround. #5406 is now on main and cargo-test is green. Closing as redundant.

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