Skip to content

Remove runtime-benchmarks feature flag from tests#3692

Open
manuelmauro wants to merge 2 commits intomasterfrom
manuel/remove-runtime-benchmarks-from-tests
Open

Remove runtime-benchmarks feature flag from tests#3692
manuelmauro wants to merge 2 commits intomasterfrom
manuel/remove-runtime-benchmarks-from-tests

Conversation

@manuelmauro
Copy link
Contributor

Summary

Removes the runtime-benchmarks feature flag from the unit test CI command (cargo test --profile testnet --workspace --features=evm-tracing,runtime-benchmarks), since it is not required for
running tests.

Motivation

The runtime-benchmarks feature was being enabled for the test suite despite no test code depending on any benchmark-gated API. Enabling it unnecessarily:

  • Changes runtime behavior under test — e.g. ExistentialDeposit goes from 0 (production value) to 1, and MessageProcessor is swapped to a NoopMessageProcessor on moonbase/moonriver, meaning tests
    aren't exercising the real runtime configuration.
  • Increases compile time — the feature cascades through 70+ SDK dependencies, compiling additional benchmark modules, helper types, and extra trait implementations that are never exercised by the
    test suite.

@manuelmauro manuelmauro self-assigned this Mar 3, 2026
@coderabbitai
Copy link

coderabbitai bot commented Mar 3, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 81dfe9e and 62cd5d8.

📒 Files selected for processing (1)
  • .github/workflows/build.yml

📝 Walkthrough

Walkthrough

The pull request modifies the CI workflow to add a dedicated compilation check for the runtime-benchmarks feature in isolation and removes this feature from the unit tests step, retaining only the evm-tracing feature for testing.

Changes

Cohort / File(s) Summary
CI/Workflow Updates
.github/workflows/build.yml
Added new cargo check step with --features=runtime-benchmarks; removed runtime-benchmarks feature from unit tests step to isolate its compilation verification.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: removing the runtime-benchmarks feature from tests, which matches the primary objective of the pull request.
Description check ✅ Passed The description is directly related to the changeset, explaining why the runtime-benchmarks feature is being removed from tests and the benefits of this change.
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
  • Post copyable unit tests in a comment
  • Commit unit tests in branch manuel/remove-runtime-benchmarks-from-tests

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.

The runtime-benchmarks feature is not needed for running tests:
- No test code uses any benchmark-gated API (try_successful_origin,
  ensure_successful, ensure_concluded, etc.)
- The #[cfg] gates in the SDK are symmetric: when the feature is off,
  the extra trait methods don't exist in the trait definitions, so
  implementations don't need to provide them either.
- All tests compile and pass without the feature.
@manuelmauro manuelmauro force-pushed the manuel/remove-runtime-benchmarks-from-tests branch from 62668dd to 75ca9ad Compare March 3, 2026 08:39
@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Coverage Report

@@                               Coverage Diff                               @@
##           master   manuel/remove-runtime-benchmarks-from-tests      +/-   ##
===============================================================================
+ Coverage   77.10%                                        77.16%   +0.06%     
  Files         389                                           389              
+ Lines       76972                                         77269     +297     
===============================================================================
+ Hits        59349                                         59622     +273     
+ Misses      17623                                         17647      +24     
Files Changed Coverage
/pallets/moonbeam-foreign-assets/src/lib.rs 83.11% (+2.50%) 🔼
/pallets/moonbeam-foreign-assets/src/mock.rs 88.64% (-0.52%) 🔽

Coverage generated Tue Mar 3 12:12:31 UTC 2026

@manuelmauro manuelmauro added B0-silent Changes should not be mentioned in any release notes D2-notlive PR doesn't change runtime code (so can't be audited) not-breaking Does not need to be mentioned in breaking changes labels Mar 3, 2026
Since runtime-benchmarks was removed from the test command, add a
dedicated cargo check step to verify the feature still compiles.
This mirrors the sole feature flag used by the benchmark cron workflow
(check-benchmarks.yml / run-benches-for-runtime.sh).
@manuelmauro manuelmauro marked this pull request as ready for review March 3, 2026 11:24
@manuelmauro manuelmauro requested review from a team as code owners March 3, 2026 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B0-silent Changes should not be mentioned in any release notes D2-notlive PR doesn't change runtime code (so can't be audited) not-breaking Does not need to be mentioned in breaking changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants