Skip to content

fix(deploy): add preflight checks for admin address and test files#1235

Open
notJoon wants to merge 1 commit intomainfrom
fix/deploy-preflight-checks
Open

fix(deploy): add preflight checks for admin address and test files#1235
notJoon wants to merge 1 commit intomainfrom
fix/deploy-preflight-checks

Conversation

@notJoon
Copy link
Copy Markdown
Member

@notJoon notJoon commented Apr 3, 2026

Problem:

  • Deploying without make patch-admin-address causes GNS to be minted to the wrong account (test1 instead of gnoswap_admin), resulting in "insufficient balance" errors during pool creation.
  • Deploying with *_test.gno files present causes gnovm deployment failures.
  • Token path variables (GNS_PATH, etc.) were missing when test.mk was
    invoked via make -f scripts/test.mk, causing "func not specified" errors.

Changes:

  • deploy.mk: add verify-admin-address and verify-no-test-files as dependencies of init target
  • test.mk: define token path variables directly so they are available regardless of invocation method

Summary by CodeRabbit

  • Chores
    • Added pre-flight verification checks to deployment process to catch configuration issues early
    • Improved test script organization with centralized package path definitions

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 3, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ef422891-436c-4f59-a0bd-a415dd3c5bca

📥 Commits

Reviewing files that changed from the base of the PR and between b0be426 and 23a7037.

📒 Files selected for processing (2)
  • tests/scripts/deploy.mk
  • tests/scripts/test.mk

Walkthrough

Two new Makefile validation targets (verify-admin-address and verify-no-test-files) were added to deploy.mk to perform pre-deployment checks. The init target now requires these validations before deploying tokens and gnoswap. Additionally, token package paths were centralized as variables in test.mk.

Changes

Cohort / File(s) Summary
Deployment Validation
tests/scripts/deploy.mk
Added verify-admin-address target to extract and validate admin address from consts.gno against ADDR_ADMIN environment variable, and verify-no-test-files target to scan for *_test.gno and testutils.gno files in contract directory. Both targets print red error messages and exit with code 1 on failure. Updated init target prerequisites to run both validations before deployment.
Token Path Configuration
tests/scripts/test.mk
Added Makefile variables to centralize external token package paths: GNS_PATH, USDC_PATH, BAZ_PATH, BAR_PATH, OBL_PATH, QUX_PATH, and FOO_PATH. Fixed trailing newline in collect-project-token target output.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and clearly describes the primary changes: adding preflight verification checks (admin address and test files) to the deployment process.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/deploy-preflight-checks

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Apr 3, 2026

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.

1 participant