Skip to content

Conversation

gustavovalverde
Copy link
Owner

Motivation

Solution

Tests

Specifications & References

Follow-up Work

PR Checklist

  • The PR name is suitable for the release notes.
  • The solution is tested.
  • The documentation is up to date.

Previously, cache mounts targeted specific subdirectories which did not match
the CARGO_HOME path, leading to ineffective caching and repeated downloads.
This change uses a single cache mount for CARGO_HOME in both test and release
stages, ensuring Cargo can leverage BuildKit's cache for faster rebuilds.

Additionally, introduced CARGO_TARGET_DIR for consistent build output paths,
replaced bind mounts with COPY --link for test stage to ensure source code
availability, and updated cache handling to preserve artifacts across builds.

Fixes ZcashFoundation#9331
Relates ZcashFoundation#6169
- Override FEATURES in Dockerfile tests stage with complete feature set needed for all tests
- Convert entrypoint.sh from cargo test to cargo nextest run with --workspace scope
- Update nextest.toml test filters to match actual function names:
  - generate_checkpoints -> generate_checkpoints_mainnet/testnet
  - fake_activation_heights -> with_fake_activation_heights
- Add nextest overrides for lightwalletd and sync test categories
- Remove feature duplication by using centralized FEATURES env var
- Align build and test scopes between Dockerfile and entrypoint to prevent recompilation

This ensures consistent feature sets across build and test phases while
standardizing on nextest for better test execution and filtering.
This commit refactors the testing architecture to remove test-specific feature flags in favor of runtime environment variables and nextest profiles. This simplifies the build process, as a single test binary can be built and used for all test scenarios.

The primary changes are:
- Test-specific features like `lightwalletd-grpc-tests`, `sync_*`, and `zebra-checkpoints` have been removed from `Cargo.toml`.
- The tests formerly guarded by these features are now controlled by `ZEBRA_TEST_*` environment variables. These act as opt-in safety guards, preventing developers from accidentally running long or resource-intensive tests locally.
- CI workflows in `.github/workflows` have been updated. They now use `nextest` profiles to select specific tests and set the corresponding `ZEBRA_TEST_*` variables to activate them.
- The `tonic-build` dependency, required for `lightwalletd` tests, is now correctly configured as a test-only build dependency. It is enabled through the `zebra-test` feature, ensuring it is not included in production release builds. This resolves a build warning related to an unknown `cfg` value.
…ntime controls

- Remove lightwalletd-grpc-tests feature from zebra-test crate
- Move lightwalletd-grpc-tests feature definition in zebrad/Cargo.toml to proper location
- Make tonic-build conditional on lightwalletd-grpc-tests feature in build.rs
- Remove feature gates from test modules, allowing them to compile unconditionally
- Remove feature gate from lightwalletd_test_suite test execution
- Update CI workflow to include lightwalletd-grpc-tests feature for comprehensive testing

Tests now use runtime environment variables (ZEBRA_TEST_*) instead of compile-time
features, allowing a single test binary to run different test suites based on
configuration while keeping production builds lean.
- Add feature gates to lightwalletd test infrastructure to prevent compilation errors when lightwalletd-grpc-tests is disabled
- Add feature gate to indexer test to prevent compilation errors when indexer is disabled
- Move lightwalletd-related imports and constants behind feature gates
- Wrap gRPC code generation in feature-conditional module
- Fix GitHub Actions workflow to pass features as single string
- Restore missing lightwalletd_failure_messages method with feature gate
- Add missing DATABASE_FORMAT_UPGRADE_IS_LONG import

This ensures --no-default-features builds work correctly while maintaining full functionality when features are enabled.
- Remove unused DATABASE_FORMAT_UPGRADE_IS_LONG import
- Update references to use common::cached_state::DATABASE_FORMAT_UPGRADE_IS_LONG
- Fix unused import linting warning
gustavovalverde and others added 27 commits August 21, 2025 15:59
- Fail config loading when sensitive env keys are present (password, secret, token, cookie, private_key)
- Update tests to assert error behavior
- Document security behavior in ZebradConfig::load
Co-authored-by: Arya <aryasolhi@gmail.com>
- Add `config = { version = "0.15.14", features = ["toml", "json"] }` to `[workspace.dependencies]`
- Switch `zebrad` to `config = { workspace = true }`
Add `ZebradConfig::load_with_env(config_path, env_prefix)` to allow loading a
config from environment variables using a caller-specified prefix. `load(...)`
delegates to this with the default `ZEBRA` prefix so existing behavior is
unchanged.

Update the `copy-state` command to load its target config with the
`ZEBRA_TARGET_...` prefix (e.g. `ZEBRA_TARGET_STATE__CACHE_DIR`) so source and
target environment overrides do not conflict.
- Adds `ZebradConfig::load_with_env` to load configs using a specified
  environment variable prefix.
- Updates the `copy-state` command to use the `ZEBRA_TARGET_` prefix for
  its target configuration, preventing conflicts with the source config's
  `ZEBRA_` environment variables.
- Improves test reliability by handling mutex poisoning in `EnvGuard`,
  which prevents a single test failure from cascading.
- Fixes a bug where environment variable overrides were not being correctly
  applied in tests due to incorrect prefix handling.
- Updates documentation and test profiles to reflect these changes.
…oundation#9830)

* Removes `ZEBRA_NETWORK__NETWORK` env var from `Run all tests`

* removes duplicate 'and'
Co-authored-by: Arya <aryasolhi@gmail.com>
Split `sync-to-mandatory-checkpoint` nextest profile into
`sync-to-mandatory-checkpoint-mainnet` and
`sync-to-mandatory-checkpoint-testnet` so each profile runs a single
network-specific test.

Update CI job `sub-ci-integration-tests-gcp.yml`
to select `NEXTEST_PROFILE` dynamically per network so only the matching
profile runs in a workflow invocation. This prevents concurrent tests for
different networks from sharing the same cache dir and creating race conditions.
- Add `ZEBRA_ENV_PREFIX` const and use it in `EnvGuard::new` and the `Drop` impl.
- Replace `assert!(result.is_err())` with `.expect_err(...)` in tests that don't inspect the error value.
Co-authored-by: Arya <aryasolhi@gmail.com>
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zizmor found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

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.

2 participants