Skip to content

[codex] Fix Pinocchio memo JS test fixtures#387

Merged
joncinque merged 2 commits into
solana-program:mainfrom
onspeedhp:codex/fix-pinocchio-js-tests
Jun 5, 2026
Merged

[codex] Fix Pinocchio memo JS test fixtures#387
joncinque merged 2 commits into
solana-program:mainfrom
onspeedhp:codex/fix-pinocchio-js-tests

Conversation

@onspeedhp

@onspeedhp onspeedhp commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix the JS test and local validator fixtures that still referenced the pre-migration spl_memo.so artifact after the program crate was replaced by the Pinocchio-based implementation.

This updates the test fixture and docs to use target/deploy/pinocchio_memo_program.so, and adjusts the JS log assertion for the Pinocchio memo log shape.

Why

The current JS client tests fail locally after build-sbf-program because the checked-in setup tries to load target/deploy/spl_memo.so, but the current program crate emits pinocchio_memo_program.so. Once pointed at the correct binary, the test still expected the legacy one-line memo log format rather than the Pinocchio two-line log output.

Validation

  • cargo build-sbf --manifest-path program/Cargo.toml --tools-version v1.54
  • SBF_OUT_DIR=/Users/mac/Documents/Private\ Payment/memo/target/deploy cargo test --workspace
  • cargo clippy --workspace --all-targets -- --deny warnings
  • cargo fmt --all --check
  • pnpm build in clients/js
  • pnpm test in clients/js
  • pnpm lint in clients/js
  • NODE_OPTIONS="$NODE_OPTIONS --experimental-vm-modules" pnpm exec jest test/unit --watchman=false in clients/js-legacy

Note: legacy validator e2e could not be completed in the Codex command runner because the background validator process does not survive across tool invocations here; the script now points at the current Pinocchio artifact.

@onspeedhp onspeedhp marked this pull request as ready for review June 5, 2026 08:19

@joncinque joncinque left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great to me, thanks for fixing this!

Can you just revert the oxfmt-related changes? Those should belong in a separate PR, if they're really needed

cc @febo

@onspeedhp

Copy link
Copy Markdown
Contributor Author

Done, reverted the oxfmt-related changes and kept this PR scoped to the Pinocchio fixture/docs updates. Thanks!

@joncinque joncinque left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

@joncinque joncinque merged commit 9238228 into solana-program:main Jun 5, 2026
28 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