From 2ecbb7f90bcf67c635a68eccdaec5e5eadf72b08 Mon Sep 17 00:00:00 2001 From: Daniel Ramirez-Chiquillo Date: Wed, 10 Sep 2025 08:52:34 -0500 Subject: [PATCH] Remove cargo test targets, use nextest exclusively (#7874) Fixes #7835 - Remove cargo test-based Make targets (`test-release`, `test-debug`, `run-ef-tests`) - Update aliases (`test`, `test-full`, `test-ef`) to use existing nextest equivalents - Update contributing documentation to use nextest examples - Fix example commands that previously referenced non-existing packages (`ssz`/`eth2_ssz`) Co-Authored-By: Daniel Ramirez-Chiquillo --- .github/workflows/test-suite.yml | 8 +-- CLAUDE.md | 51 ++++++++++++--- Makefile | 28 ++------- book/src/contributing_setup.md | 105 +++++++++---------------------- wordlist.txt | 1 + 5 files changed, 79 insertions(+), 114 deletions(-) diff --git a/.github/workflows/test-suite.yml b/.github/workflows/test-suite.yml index faa2745f55..59a045c7d3 100644 --- a/.github/workflows/test-suite.yml +++ b/.github/workflows/test-suite.yml @@ -102,7 +102,7 @@ jobs: with: version: nightly-ca67d15f4abd46394b324c50e21e66f306a1162d - name: Run tests in release - run: make nextest-release + run: make test-release - name: Show cache stats if: env.SELF_HOSTED_RUNNERS == 'true' continue-on-error: true @@ -134,7 +134,7 @@ jobs: - name: Set LIBCLANG_PATH run: echo "LIBCLANG_PATH=$((gcm clang).source -replace "clang.exe")" >> $env:GITHUB_ENV - name: Run tests in release - run: make nextest-release + run: make test-release - name: Show cache stats if: env.SELF_HOSTED_RUNNERS == 'true' continue-on-error: true @@ -269,7 +269,7 @@ jobs: with: version: nightly-ca67d15f4abd46394b324c50e21e66f306a1162d - name: Run tests in debug - run: make nextest-debug + run: make test-debug - name: Show cache stats if: env.SELF_HOSTED_RUNNERS == 'true' continue-on-error: true @@ -306,7 +306,7 @@ jobs: cache-target: release bins: cargo-nextest - name: Run consensus-spec-tests with blst and fake_crypto - run: make nextest-ef + run: make test-ef - name: Show cache stats if: env.SELF_HOSTED_RUNNERS == 'true' continue-on-error: true diff --git a/CLAUDE.md b/CLAUDE.md index 53a4433747..3e9ab169f3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -7,25 +7,28 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co **Important**: Always branch from `unstable` and target `unstable` when creating pull requests. ### Building and Installation + - `make install` - Build and install the main Lighthouse binary in release mode - `make install-lcli` - Build and install the `lcli` utility binary - `cargo build --release` - Standard Rust release build - `cargo build --bin lighthouse --features "gnosis,slasher-lmdb"` - Build with specific features ### Testing + - `make test` - Run the full test suite in release mode (excludes EF tests, beacon_chain, slasher, network, http_api) -- `make nextest-release` - Run tests using nextest (faster parallel test runner) +- `make test-release` - Run tests using nextest (faster parallel test runner) - `make test-beacon-chain` - Run beacon chain tests for all supported forks - `make test-slasher` - Run slasher tests with all database backend combinations - `make test-ef` - Download and run Ethereum Foundation test vectors - `make test-full` - Complete test suite including linting, EF tests, and execution engine tests -- `cargo test -p ` - Run tests for a specific package -- `cargo test -p ` - Run individual test (preferred during development iteration) +- `cargo nextest run -p ` - Run tests for a specific package +- `cargo nextest run -p ` - Run individual test (preferred during development iteration) - `FORK_NAME=electra cargo nextest run -p beacon_chain` - Run tests for specific fork **Note**: Full test suite takes ~20 minutes. When iterating, prefer running individual tests. -### Linting and Code Quality +### Linting and Code Quality + - `make lint` - Run Clippy linter with project-specific rules - `make lint-full` - Run comprehensive linting including tests (recommended for thorough checking) - `make cargo-fmt` - Check code formatting with rustfmt @@ -33,8 +36,9 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co - `make audit` - Run security audit on dependencies ### Cross-compilation + - `make build-x86_64` - Cross-compile for x86_64 Linux -- `make build-aarch64` - Cross-compile for ARM64 Linux +- `make build-aarch64` - Cross-compile for ARM64 Linux - `make build-riscv64` - Cross-compile for RISC-V 64-bit Linux ## Architecture Overview @@ -44,13 +48,15 @@ Lighthouse is a modular Ethereum consensus client with two main components: ### Core Components **Beacon Node** (`beacon_node/`) + - Main consensus client that syncs with the Ethereum network - Contains the beacon chain state transition logic (`beacon_node/beacon_chain/`) - Handles networking, storage, and P2P communication - Provides HTTP API for validator clients and external tools - Entry point: `beacon_node/src/lib.rs` -**Validator Client** (`validator_client/`) +**Validator Client** (`validator_client/`) + - Manages validator keystores and performs validator duties - Connects to beacon nodes via HTTP API - Handles block proposals, attestations, and sync committee duties @@ -60,31 +66,37 @@ Lighthouse is a modular Ethereum consensus client with two main components: ### Key Subsystems **Consensus Types** (`consensus/types/`) + - Core Ethereum consensus data structures (BeaconState, BeaconBlock, etc.) - Ethereum specification implementations for different networks (mainnet, gnosis) - SSZ encoding/decoding and state transition primitives **Storage** (`beacon_node/store/`) + - Hot/cold database architecture for efficient beacon chain storage - Supports multiple backends (LevelDB, RocksDB, REDB) - Handles state pruning and historical data management **Networking** (`beacon_node/lighthouse_network/`, `beacon_node/network/`) + - Libp2p-based P2P networking stack - Gossipsub for message propagation - Discovery v5 for peer discovery - Request/response protocols for sync **Fork Choice** (`consensus/fork_choice/`, `consensus/proto_array/`) + - Implements Ethereum's fork choice algorithm (proto-array) - Manages chain reorganizations and finality **Execution Layer Integration** (`beacon_node/execution_layer/`) + - Interfaces with execution clients - Retrieves payloads from local execution layer or external block builders - Handles payload validation and builder integration **Slasher** (`slasher/`) + - Optional slashing detection service - Supports LMDB, MDBX, and REDB database backends - Can be enabled with `--slasher` flag @@ -120,6 +132,7 @@ Lighthouse is a modular Ethereum consensus client with two main components: ## Common Review Standards ### CI/Testing Requirements + - All checks must pass before merge - Test coverage expected for significant changes - Flaky tests are actively addressed and fixed @@ -127,12 +140,14 @@ Lighthouse is a modular Ethereum consensus client with two main components: - `beacon_chain` and `http_api` tests support fork-specific testing using `FORK_NAME` env var when `beacon_chain/fork_from_env` feature is enabled ### Code Quality Standards + - Clippy warnings must be fixed promptly (multiple PRs show this pattern) - Code formatting with `cargo fmt` enforced - Must run `cargo sort` when adding dependencies - dependency order is enforced on CI - Performance considerations for hot paths ### Documentation and Context + - PRs require clear descriptions of what and why - Breaking changes need migration documentation - API changes require documentation updates @@ -140,6 +155,7 @@ Lighthouse is a modular Ethereum consensus client with two main components: - Comments appreciated for complex logic ### Security and Safety + - Careful review of consensus-critical code paths - Error handling patterns must be comprehensive - Input validation for external data @@ -147,6 +163,7 @@ Lighthouse is a modular Ethereum consensus client with two main components: ## Development Patterns and Best Practices ### Panics and Error Handling + - **Panics should be avoided at all costs** - Always prefer returning a `Result` or `Option` over causing a panic (e.g., prefer `array.get(1)?` over `array[1]`) - Avoid `expect` or `unwrap` at runtime - only acceptable during startup when validating CLI flags or configurations @@ -154,18 +171,22 @@ Lighthouse is a modular Ethereum consensus client with two main components: - Use proper error handling with `Result` types and graceful error propagation ### Rayon Usage + - Avoid using the rayon global thread pool as it results in CPU oversubscription when the beacon processor has fully allocated all CPUs to workers - Use scoped rayon pools started by beacon processor for computational intensive tasks ### Locks + - Take great care to avoid deadlocks when working with fork choice locks - seek detailed review ([reference](beacon_node/beacon_chain/src/canonical_head.rs:9)) - Keep lock scopes as narrow as possible to avoid blocking fast-responding functions like the networking stack ### Async Patterns + - Avoid blocking computations in async tasks - Spawn a blocking task instead for CPU-intensive work ### Tracing + - Design spans carefully and avoid overuse of spans just to add context data to events - Avoid using spans on simple getter methods as it can result in performance overhead - Be cautious of span explosion with recursive functions @@ -173,14 +194,17 @@ Lighthouse is a modular Ethereum consensus client with two main components: - Avoid using `span.enter()` or `span.entered()` in async tasks ### Database + - Maintain schema continuity on `unstable` branch - Database migrations must be backward compatible ### Consensus Crate + - Use safe math methods like `saturating_xxx` or `checked_xxx` - Critical that this crate behaves deterministically and MUST not have undefined behavior ### Testing Patterns + - **Use appropriate test types for the right scenarios**: - **Unit tests** for single component edge cases and isolated logic - **Integration tests** using [`BeaconChainHarness`](beacon_node/beacon_chain/src/test_utils.rs:668) for end-to-end workflows @@ -204,6 +228,7 @@ Lighthouse is a modular Ethereum consensus client with two main components: - See [`scripts/local_testnet/README.md`](scripts/local_testnet/README.md) for setup instructions ### TODOs and Comments + - All `TODO` statements must be accompanied by a GitHub issue link - Prefer line (`//`) comments to block comments (`/* ... */`) - Use doc comments (`///`) before attributes for public items @@ -211,7 +236,9 @@ Lighthouse is a modular Ethereum consensus client with two main components: - Provide examples in doc comments for public APIs when helpful ## Logging Guidelines + Use appropriate log levels for different scenarios: + - **`crit`**: Critical issues with major impact to Lighthouse functionality - Lighthouse may not function correctly without resolving. Needs immediate attention. - **`error`**: Error cases that may have moderate impact to Lighthouse functionality. Expect to receive reports from users for this level. - **`warn`**: Unexpected code paths that don't have major impact - fully recoverable. Expect user reports if excessive warning logs occur. @@ -221,6 +248,7 @@ Use appropriate log levels for different scenarios: ## Code Examples ### Safe Math in Consensus Crate + ```rust // ❌ Avoid - could panic let result = a + b; @@ -234,6 +262,7 @@ let result = a.safe_add(b)?; ``` ### Panics and Error Handling + ```rust // ❌ Avoid - could panic at runtime let value = some_result.unwrap(); @@ -253,6 +282,7 @@ let item = array.get(1).expect("Array always has at least 2 elements due to vali ``` ### TODO Format + ```rust pub fn my_function(&mut self, _something: &[u8]) -> Result { // TODO: Implement proper validation here @@ -261,6 +291,7 @@ pub fn my_function(&mut self, _something: &[u8]) -> Result { ``` ### Async Task Spawning for Blocking Work + ```rust // ❌ Avoid - blocking in async context async fn some_handler() { @@ -276,6 +307,7 @@ async fn some_handler() { ``` ### Tracing Span Usage + ```rust // ❌ Avoid - span on simple getter #[instrument] @@ -291,9 +323,10 @@ async fn process_block(&self, block: Block) -> Result<(), Error> { ``` ## Build and Development Notes -- Full builds and tests take 5+ minutes - use large timeouts (300s+) for any `cargo build`, `cargo test`, or `make` commands + +- Full builds and tests take 5+ minutes - use large timeouts (300s+) for any `cargo build`, `cargo nextest`, or `make` commands - Use `cargo check` for faster iteration during development and always run after code changes +- Prefer targeted package tests (`cargo nextest run -p `) and individual tests over full test suite when debugging specific issues - Use `cargo fmt --all && make lint-fix` to format code and fix linting issues once a task is complete -- Prefer targeted package tests (`cargo test -p `) and individual tests over full test suite when debugging specific issues - Always understand the broader codebase patterns before making changes -- Minimum Supported Rust Version (MSRV) is documented in `lighthouse/Cargo.toml` - ensure Rust version meets or exceeds this requirement \ No newline at end of file +- Minimum Supported Rust Version (MSRV) is documented in `lighthouse/Cargo.toml` - ensure Rust version meets or exceeds this requirement diff --git a/Makefile b/Makefile index 475d3aac8a..79fe7ea496 100644 --- a/Makefile +++ b/Makefile @@ -139,29 +139,18 @@ build-release-tarballs: $(call tarball_release_binary,$(BUILD_PATH_RISCV64),$(RISCV64_TAG),"") + # Runs the full workspace tests in **release**, without downloading any additional # test vectors. test-release: - cargo test --workspace --release --features "$(TEST_FEATURES)" \ - --exclude ef_tests --exclude beacon_chain --exclude slasher --exclude network \ - --exclude http_api - -# Runs the full workspace tests in **release**, without downloading any additional -# test vectors, using nextest. -nextest-release: cargo nextest run --workspace --release --features "$(TEST_FEATURES)" \ --exclude ef_tests --exclude beacon_chain --exclude slasher --exclude network \ --exclude http_api + # Runs the full workspace tests in **debug**, without downloading any additional test # vectors. test-debug: - cargo test --workspace --features "$(TEST_FEATURES)" \ - --exclude ef_tests --exclude beacon_chain --exclude network --exclude http_api - -# Runs the full workspace tests in **debug**, without downloading any additional test -# vectors, using nextest. -nextest-debug: cargo nextest run --workspace --features "$(TEST_FEATURES)" \ --exclude ef_tests --exclude beacon_chain --exclude network --exclude http_api @@ -173,15 +162,9 @@ cargo-fmt: check-benches: cargo check --workspace --benches --features "$(TEST_FEATURES)" -# Runs only the ef-test vectors. -run-ef-tests: - rm -rf $(EF_TESTS)/.accessed_file_log.txt - cargo test --release -p ef_tests --features "ef_tests,$(EF_TEST_FEATURES)" - cargo test --release -p ef_tests --features "ef_tests,$(EF_TEST_FEATURES),fake_crypto" - ./$(EF_TESTS)/check_all_files_accessed.py $(EF_TESTS)/.accessed_file_log.txt $(EF_TESTS)/consensus-spec-tests -# Runs EF test vectors with nextest -nextest-run-ef-tests: +# Runs EF test vectors +run-ef-tests: rm -rf $(EF_TESTS)/.accessed_file_log.txt cargo nextest run --release -p ef_tests --features "ef_tests,$(EF_TEST_FEATURES)" cargo nextest run --release -p ef_tests --features "ef_tests,$(EF_TEST_FEATURES),fake_crypto" @@ -233,9 +216,6 @@ test-ef: make-ef-tests run-ef-tests # Downloads and runs the nightly EF test vectors. test-ef-nightly: make-ef-tests-nightly run-ef-tests -# Downloads and runs the EF test vectors with nextest. -nextest-ef: make-ef-tests nextest-run-ef-tests - # Runs tests checking interop between Lighthouse and execution clients. test-exec-engine: make -C $(EXECUTION_ENGINE_INTEGRATION) test diff --git a/book/src/contributing_setup.md b/book/src/contributing_setup.md index 7143c8f0fb..b817faad87 100644 --- a/book/src/contributing_setup.md +++ b/book/src/contributing_setup.md @@ -26,7 +26,7 @@ you can run them locally and avoid CI failures: - `$ make cargo-fmt`: (fast) runs a Rust code formatting check. - `$ make lint`: (fast) runs a Rust code linter. -- `$ make test`: (medium) runs unit tests across the whole project. +- `$ make test`: (medium) runs unit tests across the whole project using nextest. - `$ make test-ef`: (medium) runs the Ethereum Foundation test vectors. - `$ make test-full`: (slow) runs the full test suite (including all previous commands). This is approximately everything @@ -36,88 +36,39 @@ _The lighthouse test suite is quite extensive, running the whole suite may take ## Testing -As with most other Rust projects, Lighthouse uses `cargo test` for unit and -integration tests. For example, to test the `ssz` crate run: +Lighthouse uses `cargo nextest` for unit and integration tests. Nextest provides better parallelization and is used by CI. For example, to test the `safe_arith` crate run: ```bash -$ cd consensus/ssz -$ cargo test - Finished test [unoptimized + debuginfo] target(s) in 7.69s - Running unittests (target/debug/deps/ssz-61fc26760142b3c4) - -running 27 tests -test decode::impls::tests::awkward_fixed_length_portion ... ok -test decode::impls::tests::invalid_h256 ... ok - -test encode::tests::test_encode_length ... ok -test encode::impls::tests::vec_of_vec_of_u8 ... ok -test encode::tests::test_encode_length_above_max_debug_panics - should panic ... ok - -test result: ok. 27 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s - - Running tests/tests.rs (target/debug/deps/tests-f8fb1f9ccb197bf4) - -running 20 tests -test round_trip::bool ... ok -test round_trip::first_offset_skips_byte ... ok -test round_trip::fixed_len_excess_bytes ... ok - -test round_trip::vec_u16 ... ok -test round_trip::vec_of_vec_u16 ... ok - -test result: ok. 20 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s - - Doc-tests ssz - -running 3 tests -test src/decode.rs - decode::SszDecoder (line 258) ... ok -test src/encode.rs - encode::SszEncoder (line 57) ... ok -test src/lib.rs - (line 10) ... ok - -test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.15s$ cargo test -p eth2_ssz +$ cd consensus/safe_arith +$ cargo nextest run + Finished test [unoptimized + debuginfo] target(s) in 0.43s + ------------ + Nextest run ID: 01234567-89ab-cdef-0123-456789abcdef + Starting 8 tests across 1 binary + PASS [ 0.001s] safe_arith tests::test_safe_add_u64 + PASS [ 0.001s] safe_arith tests::test_safe_mul_u64 + + ------------ + Summary [ 0.012s] 8 tests run: 8 passed, 0 skipped ``` -Alternatively, since `lighthouse` is a cargo workspace you can use `-p eth2_ssz` where -`eth2_ssz` is the package name as defined `/consensus/ssz/Cargo.toml` +Alternatively, since `lighthouse` is a cargo workspace you can use `-p safe_arith` where +`safe_arith` is the package name as defined in `/consensus/safe_arith/Cargo.toml`: ```bash -$ head -2 consensus/ssz/Cargo.toml +$ head -2 consensus/safe_arith/Cargo.toml [package] -name = "eth2_ssz" -$ cargo test -p eth2_ssz - Finished test [unoptimized + debuginfo] target(s) in 7.69s - Running unittests (target/debug/deps/ssz-61fc26760142b3c4) - -running 27 tests -test decode::impls::tests::awkward_fixed_length_portion ... ok -test decode::impls::tests::invalid_h256 ... ok - -test encode::tests::test_encode_length ... ok -test encode::impls::tests::vec_of_vec_of_u8 ... ok -test encode::tests::test_encode_length_above_max_debug_panics - should panic ... ok - -test result: ok. 27 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s - - Running tests/tests.rs (target/debug/deps/tests-f8fb1f9ccb197bf4) - -running 20 tests -test round_trip::bool ... ok -test round_trip::first_offset_skips_byte ... ok -test round_trip::fixed_len_excess_bytes ... ok - -test round_trip::vec_u16 ... ok -test round_trip::vec_of_vec_u16 ... ok - -test result: ok. 20 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s - - Doc-tests ssz - -running 3 tests -test src/decode.rs - decode::SszDecoder (line 258) ... ok -test src/encode.rs - encode::SszEncoder (line 57) ... ok -test src/lib.rs - (line 10) ... ok - -test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.15s$ cargo test -p eth2_ssz +name = "safe_arith" +$ cargo nextest run -p safe_arith + Finished test [unoptimized + debuginfo] target(s) in 0.43s + ------------ + Nextest run ID: 01234567-89ab-cdef-0123-456789abcdef + Starting 8 tests across 1 binary + PASS [ 0.001s] safe_arith tests::test_safe_add_u64 + PASS [ 0.001s] safe_arith tests::test_safe_mul_u64 + + ------------ + Summary [ 0.012s] 8 tests run: 8 passed, 0 skipped ``` ### test_logger @@ -129,7 +80,7 @@ testing the logs are displayed. This can be very helpful while debugging tests. Example: ``` -$ cargo test -p beacon_chain validator_pubkey_cache::test::basic_operation --features 'logging/test_logger' +$ cargo nextest run -p beacon_chain -E 'test(validator_pubkey_cache::test::basic_operation)' --features 'logging/test_logger' Finished test [unoptimized + debuginfo] target(s) in 0.20s Running unittests (target/debug/deps/beacon_chain-975363824f1143bc) diff --git a/wordlist.txt b/wordlist.txt index 0391af78cb..57674cf974 100644 --- a/wordlist.txt +++ b/wordlist.txt @@ -187,6 +187,7 @@ namespace natively nd ness +nextest nginx nitty oom