-
Notifications
You must be signed in to change notification settings - Fork 43
fix: default fee payer tests to testnet image #363
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🚀 Deploying struong/use-testnet-tempo-img with ⚡ Cloudflare Pages
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6 files reviewed, 1 comment
| async function getCurrentTempoTestnetTag(): Promise<string> { | ||
| const client = createPublicClient({ | ||
| chain: tempoTestnet, | ||
| transport: http(), | ||
| }) | ||
| const clientVersion = await client.request({ method: 'web3_clientVersion' }) | ||
| // clientVersion format: "tempo/v0.8.0-6318f1a/x86_64-unknown-linux-gnu" | ||
| const sha = clientVersion.split('/')[1].split('-').pop() | ||
| return `sha-${sha}` | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: wrap this in a try/catch to handle network failures gracefully, e.g., fallback to 'latest' if testnet RPC is unreachable
| async function getCurrentTempoTestnetTag(): Promise<string> { | |
| const client = createPublicClient({ | |
| chain: tempoTestnet, | |
| transport: http(), | |
| }) | |
| const clientVersion = await client.request({ method: 'web3_clientVersion' }) | |
| // clientVersion format: "tempo/v0.8.0-6318f1a/x86_64-unknown-linux-gnu" | |
| const sha = clientVersion.split('/')[1].split('-').pop() | |
| return `sha-${sha}` | |
| } | |
| async function getCurrentTempoTestnetTag(): Promise<string> { | |
| try { | |
| const client = createPublicClient({ | |
| chain: tempoTestnet, | |
| transport: http(), | |
| }) | |
| const clientVersion = await client.request({ method: 'web3_clientVersion' }) | |
| // clientVersion format: "tempo/v0.8.0-6318f1a/x86_64-unknown-linux-gnu" | |
| const sha = clientVersion.split('/')[1].split('-').pop() | |
| return `sha-${sha}` | |
| } catch (error) { | |
| console.warn('[getCurrentTempoTestnetTag] Failed to fetch testnet version, falling back to latest:', error) | |
| return 'latest' | |
| } | |
| } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/fee-payer/test/setup.global.ts
Line: 5:14
Comment:
**style:** wrap this in a try/catch to handle network failures gracefully, e.g., fallback to 'latest' if testnet RPC is unreachable
```suggestion
async function getCurrentTempoTestnetTag(): Promise<string> {
try {
const client = createPublicClient({
chain: tempoTestnet,
transport: http(),
})
const clientVersion = await client.request({ method: 'web3_clientVersion' })
// clientVersion format: "tempo/v0.8.0-6318f1a/x86_64-unknown-linux-gnu"
const sha = clientVersion.split('/')[1].split('-').pop()
return `sha-${sha}`
} catch (error) {
console.warn('[getCurrentTempoTestnetTag] Failed to fetch testnet version, falling back to latest:', error)
return 'latest'
}
}
```
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.* fix: dotenv to load .env into vitest * fix: use correct env path in setup.global.ts * fix: fee payer tests default to testnet image --------- Co-authored-by: Steven Truong <[email protected]>
motivation
Follows #359 with an approach that defaults the fee payer tests to use the testnet Tempo image
change
TEMPO_TAGis passeddotenvas a dev dep so that variables load into the vitest setuptesting
These are test changes
Greptile Summary
Improved test setup to default to current testnet Tempo image instead of 'latest', allowing tests to run against stable versions matching the testnet environment.
Key changes:
getCurrentTempoTestnetTag()helper that queries testnet RPC to fetch the current image SHATEMPO_TAGenv var is explicitly provideddotenvdependency to load environment variables into vitestTEMPO_TAG: latestand switching back to standard test commandprool.tsby removing unused RPC URL logicConfidence Score: 4/5
apps/fee-payer/test/setup.global.ts- consider adding error handling for the testnet RPC callImportant Files Changed
getCurrentTempoTestnetTag()helper that queries testnet RPC for current image SHA, now defaults to testnet image instead of 'latest'tagparameter directly increateServer(), removed unusedrpcUrllogicdotenv/configimport to load environment variables into processTEMPO_TAG: latestenv var and switched fromtest:testnettotestcommandSequence Diagram
sequenceDiagram participant CI as GitHub Actions participant Vitest as Vitest Runner participant Setup as globalSetup participant RPC as Tempo Testnet RPC participant Prool as Prool/Docker participant Tests as Test Suite CI->>Vitest: Run pnpm test Note over Vitest: Load dotenv/config Vitest->>Setup: Execute globalSetup() alt TEMPO_ENV != 'localnet' Setup->>Vitest: Skip setup, return early else TEMPO_ENV == 'localnet' (default) alt TEMPO_TAG env var set Setup->>Setup: Use TEMPO_TAG value else TEMPO_TAG not set Setup->>RPC: web3_clientVersion request RPC-->>Setup: "tempo/v0.8.0-6318f1a/..." Setup->>Setup: Parse SHA: "sha-6318f1a" end Setup->>Prool: createServer(tempoTag) Prool->>Docker: Start tempo container Docker-->>Prool: Container running Prool-->>Setup: Server instance Setup->>Setup: server.start() Setup->>Setup: waitForTempo() - retry loop Setup->>Docker: POST web3_clientVersion Docker-->>Setup: Response OK Setup-->>Vitest: Return teardown function end Vitest->>Tests: Run test suite Tests->>Docker: Test RPC calls Docker-->>Tests: Responses Tests-->>Vitest: Test results Vitest->>Setup: Execute teardown Setup->>Docker: Stop container