Feat(test)/mwa keep connection alive tests#283
Merged
Kuldotha merged 4 commits intoMay 11, 2026
Merged
Conversation
- CapabilitiesResult JSON + Response envelope - Deauthorize/GetCapabilities request shape via mock sender - IAdapterOperations reflection contract + [Preserve] - PlayerPrefs migration via MigrateLegacyPrefKeys reflection Refs magicblock-labs#269
Stable GUIDs for Unity import and cross-machine consistency.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds new Unity EditMode NUnit tests and accompanying .meta files: interface contract tests for IAdapterOperations, JSON deserialization tests for CapabilitiesResult, PlayerPrefs migration tests for SolanaMobileWalletAdapter, and RPC request/ID-sequencing tests for MobileWalletAdapterClient. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Tests/EditMode/JsonRpc/CapabilitiesResultTests.cs.meta`:
- Around line 1-2: Restore the missing MonoImporter metadata block in the three
new .cs.meta files by adding a MonoImporter section that includes the standard
keys: externalObjects, serializedVersion, defaultReferences, executionOrder,
icon, userData, assetBundleName, and assetBundleVariant; update the files
(Tests/EditMode/JsonRpc/CapabilitiesResultTests.cs.meta,
Tests/EditMode/Contracts/IAdapterOperationsContractTests.cs.meta,
Tests/EditMode/MwaClient/MobileWalletAdapterClientLifecycleTests.cs.meta) to
match existing C# script .meta structure so Unity won’t rewrite them.
In `@Tests/EditMode/Lifecycle/SolanaMobileWalletAdapterPrefsTests.cs`:
- Around line 1-52: Current SetUp/TearDown unconditionally delete keys and can
wipe a developer's real PlayerPrefs; modify tests to snapshot any existing
values for LegacyPk, LegacyAuthToken, NewPkKey, and NewAuthTokenKey in SetUp,
then delete keys for a clean test state (use DeleteAllRelevantKeys as-is for the
interim cleanup), and in TearDown restore the saved snapshots (recreate keys
only if they existed previously, removing keys that were absent) and call
PlayerPrefs.Save(); implement storage fields on the test class to hold the
original existence flags and values and update SetUp/TearDown to use those
fields when restoring.
In `@Tests/EditMode/MwaClient/MobileWalletAdapterClientLifecycleTests.cs`:
- Around line 38-48: Replace round-trip deserialization through JsonRequest with
direct inspection of the raw JSON bytes: update DecodeLastRequest and
DecodeRequestAt to return a JObject parsed from _sender.LastMessage and
_sender.SentMessages[index] (use Encoding.UTF8.GetString(...) then
JObject.Parse), and add helper(s) like DecodeLastParamsObject or
DecodeParamsObjectAt to extract the "params" JObject; update tests that
currently read properties off JsonRequest (e.g., asserting
request.Params.Identity) to instead assert presence/absence and exact key names
("auth_token", "identity", "params", etc.) on the JObject via Property(...) to
ensure wire-format and omitted-vs-null behavior is validated.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 87f57257-d542-43cc-bb51-f29f1b25092c
📒 Files selected for processing (10)
Tests/EditMode/Contracts.metaTests/EditMode/Contracts/IAdapterOperationsContractTests.csTests/EditMode/Contracts/IAdapterOperationsContractTests.cs.metaTests/EditMode/JsonRpc/CapabilitiesResultTests.csTests/EditMode/JsonRpc/CapabilitiesResultTests.cs.metaTests/EditMode/Lifecycle.metaTests/EditMode/Lifecycle/SolanaMobileWalletAdapterPrefsTests.csTests/EditMode/Lifecycle/SolanaMobileWalletAdapterPrefsTests.cs.metaTests/EditMode/MwaClient/MobileWalletAdapterClientLifecycleTests.csTests/EditMode/MwaClient/MobileWalletAdapterClientLifecycleTests.cs.meta
- Assert Deauthorize/GetCapabilities payloads via JObject so JsonProperty drift can't hide broken wire keys - Snapshot/restore MWA PlayerPrefs keys so EditMode runs don't wipe dev state
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Contributor
Author
Zurcusa
added a commit
to Zurcusa/Solana.Unity-SDK
that referenced
this pull request
May 11, 2026
Semaphore gate: - Wrap _Login, _SignAllTransactions, SignMessage, GetCapabilities in TryAcquireGate/ReleaseGate to prevent concurrent state corruption Bug fixes: - Preserve WaitForCommitmentToSendNextTransaction when backfilling MinContextSlot - Set WalletBase.Account on LoginWithSignIn success - Default null mwaOptions in SolanaWalletAdapter so cache injection works Upstream PR magicblock-labs#283 test compat: - Update IAdapterOperationsContractTests for v2 AuthorizeAsync API - Rewrite SolanaMobileWalletAdapterPrefsTests for v2 cache-based migration - Fix MobileWalletAdapterClientLifecycleTests Authorize → AuthorizeAsync
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
PR #269 added three new lifecycle surfaces to the Mobile Wallet Adapter stack:
Deauthorize(authToken)RPC onIAdapterOperations/MobileWalletAdapterClient.GetCapabilities()RPC returning a newCapabilitiesResultresponse model.keepConnectionAlivereauthorize path plus a PlayerPrefs migration from the legacypk/authTokenkeys to namespacedsolana_sdk.mwa.public_key/solana_sdk.mwa.auth_token.None of that has automated coverage. A rename, a silent JSON property drift, or a regression in the key-migration logic would only surface during manual on-device testing, which is exactly the kind of failure the test infrastructure added in #276 was meant to prevent.
Solution
This PR adds 44 new EditMode unit tests across 4 test classes, using the
MockMessageSenderand NUnit conventions already established by #276.Coverage added:
CapabilitiesResultTests(11 tests): pins the snake_case JSON wire format (max_transactions_per_request,max_messages_per_request,supported_transaction_versions,supports_clone_authorization), verifies every nullable field staysnullwhen absent (includingSupportsCloneAuthorization, which isbool?and must not silently coerce tofalse), confirms unknown JSON fields are ignored for forward-compat with the MWA spec, and verifiesResponse<CapabilitiesResult>composes correctly with the existingWasSuccessful/Failedenvelope flags.MobileWalletAdapterClientLifecycleTests(16 tests): asserts the exact JSON-RPC request shape sent to the wire for both new RPCs: method name (deauthorize,get_capabilities), JSON-RPC version, monotonic and positive message IDs, parameter whitelist (Deauthorize must not leakidentity/cluster/payloads/addresses; GetCapabilities must send a non-null emptyParamsobject), cross-method ID sequencing, and the public return typeTask<CapabilitiesResult>.IAdapterOperationsContractTests(11 tests): reflection-based contract tests pinning every method signature on the interface. GuaranteesDeauthorizereturns the non-genericTask(fire-and-forget),GetCapabilitiesreturnsTask<CapabilitiesResult>, every method and the interface itself carry[Preserve](required so IL2CPP / Unity managed code stripping does not remove them in AOT builds), and the method count stays locked at 6.SolanaMobileWalletAdapterPrefsTests(8 tests,[Category("Lifecycle")]): pins the privateconstkey names (solana_sdk.mwa.public_key,solana_sdk.mwa.auth_token) so a rename breaks CI instead of users' existing installs, and exercises the private staticMigrateLegacyPrefKeysvia reflection. Covers: no-op when no legacy keys, migrates each legacy key independently, deletes legacy keys after migration, does not overwrite a newer namespaced value, idempotent on a second call, handles half-migrated installs.[SetUp]and[TearDown]scrub PlayerPrefs so the registry-backed store stays clean between runs.Note:
SolanaMobileWalletAdapteritself cannot be instantiated in EditMode because its constructor throws on non-Android platforms, this is the same blocker called out in the #276 PR body. Migration logic is reached through reflection to work around that without modifying runtime code.Before & After Screenshots
BEFORE:
34 passing EditMode tests (from #276). No coverage for PR #269's lifecycle surface.
AFTER:
78 passing EditMode tests in the Unity Test Runner.
Other changes (e.g. bug fixes, small refactors)
com.solana.unity_sdk,Newtonsoft.Json.dll,nunit.framework.dll, and the Unity test runners already referenced byEditMode.asmdef.Tests/EditMode/:Contracts/,Lifecycle/(plus files under the existingJsonRpc/andMwaClient/).Deploy Notes
Tooling-only. No runtime behavior change.
New scripts:
Tests/EditMode/JsonRpc/CapabilitiesResultTests.csTests/EditMode/MwaClient/MobileWalletAdapterClientLifecycleTests.csTests/EditMode/Contracts/IAdapterOperationsContractTests.csTests/EditMode/Lifecycle/SolanaMobileWalletAdapterPrefsTests.csNew dependencies:
Summary by CodeRabbit