fix(ci): rename AISIX_REDIS_URL → CACHE_TEST_REDIS_URL to unblock CI - #39
fix(ci): rename AISIX_REDIS_URL → CACHE_TEST_REDIS_URL to unblock CI#39moonming wants to merge 1 commit into
Conversation
…t tests CI on main has been red since #34 because aisix-core's Config crate loader merges every AISIX_-prefixed env var into the root Config struct (config-rs Environment::with_prefix("AISIX")), and Config has #[serde(deny_unknown_fields)]. The redis integration test sets AISIX_REDIS_URL on the rust-unit job, which leaks into every Config::load_from_path call as `redis_url` and panics: Config("deserialize: unknown field `redis_url`, expected one of `etcd`, `proxy`, `admin`, `observability`, `cache`, `managed`") 8 of 9 aisix-core::config::tests fail (the one that doesn't is rejects_unknown_fields, which intentionally swallows the error). Rename the env var so it doesn't sit under the AISIX_ prefix at all. crates/aisix-cache/tests/redis_integration.rs reads CACHE_TEST_REDIS_URL; CI sets the same. docs/testing.md + crates/aisix-cache/src/redis.rs comment updated to match. Verified: with the rename, all 9 config tests pass even with CACHE_TEST_REDIS_URL set; reproducing with the old AISIX_REDIS_URL still fails as expected (so the loader behaviour is unchanged for real AISIX_-prefixed env overrides).
There was a problem hiding this comment.
Pull request overview
Renames the Redis integration-test environment variable to avoid AISIX_* leaking into aisix-core config loading (which denies unknown fields), unblocking CI.
Changes:
- Switched Redis integration tests to read
CACHE_TEST_REDIS_URLinstead ofAISIX_REDIS_URL. - Updated GitHub Actions
rust-unitjob to setCACHE_TEST_REDIS_URL(and documented why it must not use theAISIX_prefix). - Updated testing docs and in-crate documentation to match the new env var name.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| docs/testing.md | Updates integration-test and CI documentation to reference CACHE_TEST_REDIS_URL. |
| crates/aisix-cache/tests/redis_integration.rs | Reads CACHE_TEST_REDIS_URL and updates skip messages accordingly. |
| crates/aisix-cache/src/redis.rs | Updates doc comment pointing to the opt-in integration env var. |
| .github/workflows/ci.yml | Sets CACHE_TEST_REDIS_URL for the rust-unit job and documents the config-loader interaction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
✅ Rust tests all pass with the rename — confirmed via the CI log: ``` The job is showing as failed because of: ``` That's an org-level Actions storage quota issue (independent of this PR — `actions/upload-artifact@v4` rejects regardless of test outcome). Either free up artifacts or upgrade the plan to make CI pass cleanly. The fix itself is verified end-to-end. 🤖 Generated with Claude Code |
CI on
mainhas been red since #34 becauseaisix-core'sConfigloader merges everyAISIX_*env var into the rootConfigstruct (via config-rsEnvironment::with_prefix("AISIX")), andConfighas#[serde(deny_unknown_fields)].The redis integration test sets
AISIX_REDIS_URLon therust-unitjob, which leaks into everyConfig::load_from_pathcall as a strayredis_urlfield and panics:8 / 9
aisix-core::config::testsfail (the one that passes isrejects_unknown_fields— it swallows the error intentionally).Fix
Rename the env var so it doesn't sit under the
AISIX_prefix at all:crates/aisix-cache/tests/redis_integration.rsreadsCACHE_TEST_REDIS_URL.github/workflows/ci.ymlsetsCACHE_TEST_REDIS_URLcrates/aisix-cache/src/redis.rsdoc comment +docs/testing.mdupdated to matchTest plan
cargo test -p aisix-core --lib config::tests— all 9 pass with the renameAISIX_REDIS_URL=… cargo test … loads_minimal_configstill fails (so the loader behaviour for realAISIX_*overrides is unchanged)🤖 Generated with Claude Code