Skip to content

test(arkts): fix harmonyos-smoke compile — byte_offset drift after #5250#5288

Merged
proggeramlug merged 1 commit into
mainfrom
fix/arkts-byte-offset-drift
Jun 17, 2026
Merged

test(arkts): fix harmonyos-smoke compile — byte_offset drift after #5250#5288
proggeramlug merged 1 commit into
mainfrom
fix/arkts-byte-offset-drift

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Why harmonyos-smoke is red

The harmonyos-smoke job fails at step 5 (cargo test -p perry-codegen-arkts --lib) with:

error[E0063]: missing field `byte_offset` in initializer of `perry_hir::Expr`
  crates/perry-codegen-arkts/src/tests.rs:956, :2531, :2763
  crates/perry-codegen-arkts/tests/phase2_full_app_smoke.rs:145, :156, :521

#5250 added Expr::Call.byte_offset (call-site source locations for diagnostics) but missed the 6 Expr::Call { .. } literals in perry-codegen-arkts's tests. Because perry-codegen-arkts is not built by the gating cargo-test job — only by harmonyos-smoke, which is continue-on-error: true (informational, "must not block package publish") — this E0063 has sat red on main, and on every open PR, without failing any required check. Same class as the #5266 cheerio fix (which patched the perry-hir integration test the same merge missed).

Fix

Add byte_offset: 0 to the 6 Expr::Call initializers (3 in src/tests.rs, 3 in tests/phase2_full_app_smoke.rs). The HarmonyOS/ArkTS codegen target is real, so the smoke test is worth keeping green rather than deleting — it was only bit-rotting because it's non-gating.

Verification

Exactly the two commands harmonyos-smoke runs:

  • cargo test -p perry-codegen-arkts --release --lib111 passed, 0 failed
  • cargo test -p perry-codegen-arkts --release --test phase2_full_app_smoke2 passed, 0 failed

fmt clean; file-size / GC store-site / addr-class gates pass. No version / CHANGELOG.md / Cargo.lock edits.

Summary by CodeRabbit

  • Tests
    • Updated internal test fixtures with required fields to maintain compatibility with test infrastructure.

Note: This release contains no user-facing changes.

…e compile after #5250

#5250 added `Expr::Call.byte_offset` but missed the `Expr::Call { .. }`
initializers in perry-codegen-arkts's tests (3 in src/tests.rs, 3 in
tests/phase2_full_app_smoke.rs). perry-codegen-arkts isn't built by the gating
`cargo-test` job, only by the informational `harmonyos-smoke` job
(continue-on-error), so this E0063 sat red on main — and on every open PR —
without blocking anything. Mirrors the #5266 cheerio fix.

After: `cargo test -p perry-codegen-arkts --lib` (111 passed) and
`--test phase2_full_app_smoke` (2 passed) both green. No version/CHANGELOG/Cargo.lock edits.
@coderabbitai

coderabbitai Bot commented Jun 17, 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 Plus

Run ID: 837554d2-a1be-43fe-a7e1-a2523612b28c

📥 Commits

Reviewing files that changed from the base of the PR and between d46feff and d1193f6.

📒 Files selected for processing (2)
  • crates/perry-codegen-arkts/src/tests.rs
  • crates/perry-codegen-arkts/tests/phase2_full_app_smoke.rs

📝 Walkthrough

Walkthrough

Six test fixture lines across two files (crates/perry-codegen-arkts/src/tests.rs and crates/perry-codegen-arkts/tests/phase2_full_app_smoke.rs) are updated to add a byte_offset: 0 field to Expr::PropertyGet and Expr::Call struct literals, conforming to a new required field in the HIR/IR type.

Changes

Test Fixture Update for byte_offset Field

Layer / File(s) Summary
Add byte_offset: 0 to expr struct literals
crates/perry-codegen-arkts/src/tests.rs, crates/perry-codegen-arkts/tests/phase2_full_app_smoke.rs
Adds the required byte_offset: 0 field to Expr::PropertyGet in state_method_call, two Expr::Call literals in Issue #410 tests, and three Expr::PropertyGet nodes in Phase 2 smoke tests. No assertions or emitted-source expectations are changed.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

  • PerryTS/perry#5266: Both PRs update Rust test fixtures to add the new required byte_offset: 0 field to Expr::Call and related expression struct literals so tests compile after the byte_offset field was introduced.

Poem

A bunny hops through struct fields with glee,
byte_offset: 0 — right where it should be!
No assertion changed, no logic rearranged,
Just a field slotted in, neat as can be. 🐇✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the fix: addressing a compile error in harmonyos-smoke tests caused by missing byte_offset fields after PR #5250.
Description check ✅ Passed The description comprehensively explains the root cause, impact, fix applied, and verification results, closely following the provided template structure.
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/arkts-byte-offset-drift

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

@proggeramlug proggeramlug merged commit 3426488 into main Jun 17, 2026
14 of 15 checks passed
@proggeramlug proggeramlug deleted the fix/arkts-byte-offset-drift branch June 17, 2026 03:06
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