fix: resolve CI test failures from env var race and startup HTTP calls#578
Merged
Conversation
…TTP calls Tests were failing because prepare_tests() used unsafe set_var() to set HOME/XDG_CONFIG_HOME across parallel test threads (undefined behavior). Combined with blocking HTTP calls to nearblocks API added at startup in #574, the subprocess would read wrong config and produce empty output. - Pass env vars explicitly to subprocess Command instead of set_var - Pre-create ft_contracts.json in test setup to skip startup HTTP calls - Simplify is_used_ft_contract_account_list_exist to just check file existence - Write valid empty JSON ("[]") instead of empty file on API failure
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c75bc90501
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes CI integration test flakiness caused by process-wide environment mutation during parallel tests, and avoids startup-time blocking HTTP calls to nearblocks during test runs.
Changes:
- Replace global
std::env::set_vartest setup with per-subprocess environment variables passed viaCommand::env. - Introduce a
TestContextto keep the sandbox + tempdir alive and expose the test config home path. - Adjust FT contracts cache handling to avoid repeated nearblocks fetches and write valid empty JSON (
[]) on fetch failure.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/tokens.rs | Uses TestContext and passes config env vars directly to the spawned near subprocess. |
| tests/common/mod.rs | Refactors test harness into TestContext, removes global env mutation, and pre-creates ft_contracts.json with []. |
| tests/account.rs | Updates integration tests to use TestContext and per-process env vars. |
| src/common.rs | Simplifies FT contracts list “existence” check and ensures empty JSON is written on fetch failure. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
nonexistent.near was registered on mainnet, causing the test to fail. Use a more obscure name that is unlikely to be registered.
frol
reviewed
Mar 28, 2026
…t config - Remove else branch that wrote empty [] on fetch failure, so the CLI retries on the next run instead of caching an empty list permanently - Place test config at the correct platform-specific path for macOS
frol
approved these changes
Mar 30, 2026
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
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.
Summary
test_view_account_summary_with_localnetsnapshot test gets empty stdout from thenearbinary subprocessprepare_tests()usedunsafe { std::env::set_var() }to setHOME/XDG_CONFIG_HOMEacross parallel test threads (undefined behavior in Rust). Combined with the blocking HTTP calls toapi.nearblocks.ioadded at startup in feat: Improved interactive contract selection option for FT tokens #574, the subprocess reads the wrong config directory and fails silentlyCommandvia.env()instead of relying on process-wideset_varft_contracts.jsonwith[]in test credentials dir to skip the startup HTTP call entirelyis_used_ft_contract_account_list_existto just check file existence (prevents re-fetching from API on every invocation when the file exists but is empty)[]) instead of an empty file when the nearblocks API failsTest plan
Run Testsjob passes (currently failing on main)test_view_account_summary_with_localnetsnapshot matchestest_view_account_summary_nonexistentpassestest_view_near_balancepasses