chore: Mock test data for the Instruction Cards #793

shazzzam · 2025-12-18T16:02:41Z

Description

Mock Solana RPC calls in tests to avoid 429s in the pipeline and make tests run faster in CI

This PR adds new .env variable TEST_USE_REAL_RPC=true enables real RPC calls and overwrites existing fixtures, whiles false/unset runs tests mocked with data from fixtures

Type of change

Screenshots

Testing

Related Issues

Checklist

My code follows the project's style guidelines
I have added tests that prove my fix/feature works
All tests pass locally and in CI
CI/CD checks pass

Additional Notes

Important

Add mock system for Solana RPC calls to improve test reliability and speed by avoiding rate limits.

Behavior:
- Introduces mock system for Solana RPC calls in mock-solana-rpc.ts to avoid 429 errors and speed up tests.
- Adds .env variable TEST_USE_REAL_RPC to toggle real RPC calls.
Testing:
- Updates tests in TokenExtensionRow.spec.tsx, BaseInstructionCard.spec.tsx, and AssociatedTokenDetailsCard.spec.tsx to use mock connections.
- Uses createMockConnection and createRealConnection to manage RPC call behavior.
Fixtures:
- Adds JSON fixtures for RPC responses in fixtures/rpc-responses/ to simulate RPC call data.

^{This description was created by}^{for 33be33c. You can customize this summary. It will automatically update as commits are pushed.}

vercel · 2025-12-18T16:02:46Z

@shazzzam is attempting to deploy a commit to the Solana Foundation Team on Vercel.

A member of the Team first needs to authorize it.

ellipsis-dev

Caution

Changes requested ❌

Reviewed everything up to 33be33c in 4 minutes and 2 seconds. Click for details.

Reviewed 6783 lines of code in 14 files
Skipped 0 files when reviewing.
Skipped posting 9 draft comments. View those below.
Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. app/__tests__/mock-solana-rpc.ts:64

Draft comment:
The hashParams function uses a simple bit‐shift/sum algorithm. While acceptable for tests, consider using a more robust hash to avoid potential collisions when parameter complexity increases.
Reason this comment was not posted:
Confidence changes required: 33% <= threshold 50% None

2. app/__tests__/mock-solana-rpc.ts:96

Draft comment:
The preprocessForSerialization function uses a single WeakSet to detect circular references. Note that if the same object is referenced in two different parts (shared reference), it will throw an error even if not circular. Confirm if this behavior is intended.
Reason this comment was not posted:
Confidence changes required: 33% <= threshold 50% None

3. app/components/instruction/__tests__/ComputeBudgetDetailsCard.spec.tsx:21

Draft comment:
The search params mock returns 'devnet', yet createRealConnection is called with 'mainnet-beta'. This inconsistency may lead to confusing test behavior—consider aligning the cluster names.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

4. app/__tests__/fixtures/rpc-responses/common-base-instruction-card-getAddressLookupTable-cuo2mz.json:857

Draft comment:
Typo found: The string "Sysvar1nstructions1111111111111111111111111" appears to have a '1' instead of an 'I'. Consider correcting it to "SysvarInstructions1111111111111111111111111" if that is the intended value.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 5% vs. threshold = 50% This is a test fixture file containing what appears to be real blockchain data (an address lookup table response). The string "Sysvar1nstructions1111111111111111111111111" looks like it could be a Solana sysvar account address. In Solana, there is indeed a well-known sysvar called "SysvarInstructions" but the actual on-chain address uses "1nstructions" (with a digit 1, not the letter I). This is intentional because Solana addresses use base58 encoding which doesn't include certain characters. The comment is suggesting a "fix" that would actually break the test data by changing a valid address to an invalid one. This is fixture data meant to represent actual RPC responses, so it should match reality exactly. I might be wrong about the exact Solana address format. However, this is clearly test fixture data from an RPC response, not hand-written code. Even if I'm uncertain about the exact address, changing fixture data based on what "looks like" a typo is dangerous - fixture data should match actual responses. Even if I'm not 100% certain about the Solana address format, the key point is that this is a test fixture file containing captured RPC response data. Such files should contain actual data from the system being tested, not "corrected" versions. The comment is suggesting to modify test data based on appearance, which violates the principle that test fixtures should represent reality. This comment should be deleted. It's suggesting a change to test fixture data that represents an actual RPC response. Test fixtures should contain real data as-is, not "corrected" data. The value is likely a valid Solana address where the '1' is intentional, not a typo.

5. app/__tests__/fixtures/rpc-responses/inspector-into-parse-data-getAddressLookupTable-cuo2mz.json:857

Draft comment:
Typo detected: The public key value "Sysvar1nstructions1111111111111111111111111" appears to have a numeric '1' in place of the letter 'I'. Consider correcting it to "SysvarInstructions1111111111111111111111111".
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a test fixture file containing RPC response data. The filename suggests this is captured data from an actual blockchain query ("inspector-into-parse-data-getAddressLookupTable"). If this is real data from the blockchain, then the value "Sysvar1nstructions1111111111111111111111111" would be the actual address, not a typo. Solana addresses are base58-encoded and can contain what looks like "1nstructions" - this is likely intentional. The comment assumes this is a typo, but test fixtures should preserve the actual data they're testing against. The tool is making an assumption about what the "correct" value should be without evidence. I might be wrong if this fixture file is meant to be manually created test data rather than captured real data. However, the filename pattern and structure strongly suggest this is captured RPC response data. Also, I should verify if "Sysvar1nstructions" is even a valid Solana address format - it might not be. Even if this were manually created test data, the comment doesn't provide strong evidence that this is wrong. The actual Solana sysvar instructions address is "Sysvar1nstructions1111111111111111111111111" (with a "1" not an "I"), so this appears to be correct. The comment is making an incorrect assumption about what the value should be. This comment should be deleted. It appears to be flagging the correct Solana sysvar address as a typo when it's actually correct. Test fixture files should preserve actual blockchain data, and this value matches the real Solana sysvar instructions address.

6. app/components/inspector/__tests__/InspectorPage-SystemProgram.spec.tsx:99

Draft comment:
Typographical issue: In the test description "TransactionInspectorPage with SystemProgram' instructions", there's an unexpected apostrophe after SystemProgram. Consider removing it or changing it to "SystemProgram's instructions" if that's the intended description.
Reason this comment was not posted:
Comment was on unchanged code.

7. app/components/inspector/__tests__/into-parsed-data.spec.ts:15

Draft comment:
There's a potential typographical error: the string 'inspector-into-parse-data' might be intended to be 'inspector-into-parsed-data' to match the file name and naming consistency.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 15% vs. threshold = 50% The comment is about naming consistency - suggesting the string should match the file name. However, this is a string identifier being passed to createMockConnection, not a file path or import. The author consistently used 'inspector-into-parse-data' (without 'ed') three times, which suggests this was intentional. Without seeing the implementation of createMockConnection, I can't determine if this string has any semantic meaning or if it's just an arbitrary identifier. The comment is speculative - it says "might be intended" rather than pointing to a definite bug. This violates the rule about not making speculative comments. The code will run fine either way since it's just a string identifier. Could the string 'inspector-into-parse-data' be intentionally different from the file name for a valid reason? Perhaps it's a namespace or cache key that follows different naming conventions. The consistency across all three uses suggests deliberate choice. The comment is indeed speculative ("might be intended") and doesn't point to a clear bug. The consistent use of the same string across all three tests strongly suggests this was intentional. Without evidence that this causes a problem, this is just a stylistic suggestion about naming, which doesn't meet the bar for a useful review comment. This comment should be deleted. It's speculative (uses "might be"), doesn't identify a clear bug, and the consistent usage suggests the naming was intentional. It's a minor stylistic suggestion that doesn't warrant a review comment.

8. app/components/inspector/__tests__/into-parsed-data.spec.ts:54

Draft comment:
Typo / mismatch: The string 'inspector-into-parse-data' may be intended to be 'inspector-into-parsed-data', to match the file name or intended naming.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 15% vs. threshold = 50% The comment is speculative - it says "may be intended to be" which indicates uncertainty. Without seeing the implementation of createMockConnection, I cannot determine if this string needs to match the file name or if it's just an arbitrary identifier. The string could be intentional - perhaps it's a cache key, test identifier, or namespace that doesn't need to match the file name exactly. The comment doesn't provide strong evidence that this is actually wrong. The PR author made this change deliberately across three test cases, so they likely chose this string intentionally. This seems like a minor naming consistency suggestion rather than a functional issue. I might be missing context about the createMockConnection function's requirements - perhaps it does need to match the file name for some organizational or caching reason. The consistency between the file name and the identifier could be important for debugging or test organization. Even if there's a convention to match file names, this is a very minor issue and the comment is speculative ("may be intended"). Without strong evidence that this is wrong or causes a problem, and given that the author deliberately chose this string, the comment doesn't meet the threshold for being kept. It's not clearly actionable. This comment is speculative and lacks strong evidence that the string is actually wrong. The PR author deliberately chose 'inspector-into-parse-data' across multiple test cases, suggesting it's intentional. Without knowing the requirements of createMockConnection or evidence of a problem, this is at best a minor naming suggestion that doesn't warrant a comment.

9. app/components/inspector/__tests__/into-parsed-data.spec.ts:93

Draft comment:
Typo: The string 'inspector-into-parse-data' might be a typo. Considering the file name and context, it probably should be 'inspector-into-parsed-data' for consistency.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 15% vs. threshold = 50% This comment is about a string identifier 'inspector-into-parse-data' vs 'inspector-into-parsed-data'. The file is named 'into-parsed-data.spec.ts' so there's a slight inconsistency. However, this is a string identifier being passed to createMockConnection - without seeing the implementation of createMockConnection, I can't know if this string matters functionally or if it's just a label. The author deliberately chose this string and used it consistently three times in the same PR. This could be intentional (maybe they prefer 'parse-data' over 'parsed-data'), or it could be a typo. The comment is speculative ("might be a typo") and about a very minor naming inconsistency. It's not clearly a bug - the code will work either way. This violates the rule about not making comments that are obvious or unimportant, and it's speculative rather than definitively identifying an issue. The string identifier might actually be important for the mocking system - it could be used as a cache key or identifier that needs to be consistent. If the author used the same string three times, they likely did so intentionally. Without knowing the implementation details, I can't be sure this is actually wrong. Even if the string is functionally important, the comment is still speculative ("might be a typo") and about a minor naming inconsistency. The author clearly made a deliberate choice to use this string three times consistently. This is not a clear bug or issue that requires a code change - it's at best a subjective naming preference. This comment should be deleted. It's speculative, minor, and not clearly identifying a bug. The author consistently used the same string identifier three times, suggesting it was intentional. Without strong evidence that this is actually wrong, the comment doesn't meet the threshold for being kept.

Workflow ID: wflow_wzZxfPQvJ5MXs6HG

^{You can customize}^{by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

app/__tests__/mock-solana-rpc.ts

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>

….com:hoodieshq/explorer into fix/improve-component-tests-for-instructions

rogaldh · 2025-12-23T13:39:10Z

scripts/record-cluster-fixtures.ts

+
+// Common test configurations
+const TEST_CONFIGS = {
+    'default-test': {


I think that it is better to name it explicitly the mainnet. Cluster slug from the /clusters will fit

rogaldh · 2025-12-23T13:40:20Z

scripts/record-cluster-fixtures.ts

+const TEST_CONFIGS = {
+    'default-test': {
+        cluster: 'mainnet-beta',
+        url: 'https://api.mainnet-beta.solana.com',


Let's use RPC addresses from cluster as well to. Otherwise this may drift upon changes into main RPCs

rogaldh · 2025-12-23T13:44:46Z

scripts/record-cluster-fixtures.ts

@@ -0,0 +1,158 @@
+#!/usr/bin/env tsx


I like the feature personally, but could you move this into a separate PR, please? That is to narrow the context for changes. Already existing fixtures are good to go

rogaldh · 2025-12-23T13:49:05Z

scripts/record-cluster-fixtures.ts

+            throw new Error(`getFirstAvailableBlock error: ${JSON.stringify(firstBlockData.error)}`);
+
+        // Build fixture data
+        const fixtureData = {


Let's use existing types for epoch and related ones. That is to ensure we write down proper data into fixture

rogaldh · 2025-12-23T13:51:40Z

package.json

        "sb": "storybook dev -p 6006",
        "scan": "next dev & react-scan localhost:3000",
        "start": "next start",
+        "writeclusterfixtures": "tsx scripts/record-cluster-fixtures.ts all",


Let's move docs how to run this script into docs/development.md, please. Small guide how to work with fixtures would be a good addition

rogaldh · 2025-12-23T13:54:38Z

app/__tests__/fixtures/cluster-updates/cluster-updates-devnet.json

+    "__type": "bigint",
+    "__value": "116113408"
+  }
+}


Could you, please, ensure that all fixtures are created with proper symbol in the end. Looks like fixtures are not the target for prettier atm

rogaldh · 2025-12-23T14:30:42Z

app/__tests__/mock-solana-rpc.ts

+    const filepath = join(FIXTURES_DIR, `${filename}.json`);
+    // Preprocess to catch PublicKey and other special types before JSON.stringify
+    const preprocessed = preprocessForSerialization(data);
+    writeFileSync(filepath, JSON.stringify(preprocessed, null, 2), 'utf-8');


Could we also minify the fixture?

rogaldh · 2025-12-23T14:55:28Z

app/__tests__/mock-solana-rpc.ts

+        hash = (hash << 5) - hash + char;
+    }
+
+    return `${method}-${Math.abs(hash).toString(36)}`;


That logic is quite complex. I'd suggest using WEB Crypto API to create hashes. Something like that:

const data = new TextEncoder().encode(JSON.stringify(params)); const hashBuffer = await crypto.subtle.digest('SHA-256', data); const hash = Buffer.from(hashBuffer).toString('hex').slice(0, 12); // 12 to make it more resilient to the overlap return `${method}-${hash}`;

rogaldh · 2025-12-23T15:13:15Z

...ests__/fixtures/rpc-responses/common-base-instruction-card-getAddressLookupTable-cuo2mz.json

@@ -0,0 +1,1010 @@
+{


Is there a reson we write the type for each value into fixture? I think that it is more reasonable to keep it as close to API response as possible and convert into proper type upon reading the fixture

Types are stored only for BigInt and PublicKeys, they are needed to restore object using corresponding constructors

rogaldh · 2025-12-23T15:22:10Z

app/components/account/__tests__/TokenExtensionRow.spec.tsx

 import { TokenExtensionRow } from '../TokenAccountSection';

+vi.mock('@solana/web3.js', async () => {
+    const actual = await vi.importActual<typeof import('@solana/web3.js')>('@solana/web3.js');


Could we import the module via * wildcard at the top and use it as const actual = await vi.importActual<typeof SolanaKit>();?

rogaldh · 2025-12-23T15:27:06Z

app/__tests__/mock-solana-rpc.ts

+};
+
+export function createRealConnection(
+    cluster: 'mainnet-beta' | 'testnet' | 'devnet' = 'mainnet-beta'


I think it's better to use already existing aliases for clusters.

rogaldh · 2025-12-23T15:31:38Z

app/__tests__/mock-solana-rpc.ts

+}
+
+export function createMockConnection(testName: string, realConnection?: Connection): Connection {
+    if (USE_REAL_RPC && !realConnection) {


Could we, please, move the detection layer for RPC to a top-level component to separate core logic from the check whether is it possible to use it?

rogaldh · 2025-12-23T15:40:11Z

app/components/common/__tests__/BaseInstructionCard.spec.tsx

        const m = mock.deserializeMessageV0(stubs.aTokenCreateIdempotentMsg);
-        const connection = new Connection(clusterApiUrl('mainnet-beta'));
+        const realConnection = createRealConnection('mainnet-beta');
+        const connection = createMockConnection('common-base-instruction-card', realConnection);


What do you think we change drift from the <%test-case-name%> to <%epoch_name%> to create fixtures?
In case we have two different tests created at the same time, they will produce separate fixtures. Correct me if I am wrong here.
That moves the focus from the current cluster state (epoch, for example) to the component and produces extra fixtures likely.

rogaldh

Let's mock responses for Instruciton cards first and add fixture creation within separate PR

add mocks and fixtures for solana rpc calls

33be33c

ellipsis-dev bot reviewed Dec 18, 2025

View reviewed changes

app/__tests__/mock-solana-rpc.ts Outdated Show resolved Hide resolved

shazzzam and others added 2 commits December 18, 2025 21:21

fix prettier issues

3b796fb

Update app/__tests__/mock-solana-rpc.ts

25d84b5

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>

shazzzam changed the title ~~Mock test data for the Instruction Cards~~ chore: Mock test data for the Instruction Cards Dec 18, 2025

shazzzam added 2 commits December 19, 2025 17:59

add solana rpc calls mock to cluster provider

f99c272

Merge branch 'fix/improve-component-tests-for-instructions' of github…

8e9a9c6

….com:hoodieshq/explorer into fix/improve-component-tests-for-instructions

rogaldh reviewed Dec 23, 2025

View reviewed changes

rogaldh suggested changes Dec 23, 2025

View reviewed changes

shazzzam marked this pull request as draft December 23, 2025 17:07

chore: Mock test data for the Instruction Cards #793

Are you sure you want to change the base?

chore: Mock test data for the Instruction Cards #793

Uh oh!

Conversation

shazzzam commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Screenshots

Testing

Related Issues

Checklist

Additional Notes

Uh oh!

vercel bot commented Dec 18, 2025

Uh oh!

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rogaldh Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rogaldh left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

shazzzam commented Dec 18, 2025 •

edited

Loading

rogaldh Dec 23, 2025 •

edited

Loading