Skip to content

fix: split buildtest into a unit-test pass and a separate benchmark pass#724

Merged
credfeto merged 2 commits into
mainfrom
fix/723-buildtest-remove-invalid-test-flags
Jul 16, 2026
Merged

fix: split buildtest into a unit-test pass and a separate benchmark pass#724
credfeto merged 2 commits into
mainfrom
fix/723-buildtest-remove-invalid-test-flags

Conversation

@dnyw4l3n13

@dnyw4l3n13 dnyw4l3n13 commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • development/buildtest now mirrors credfeto-global-pre-commit's src/scripts/buildtest design: the main dotnet test run always excludes every benchmark test project (keeping --long-running/--parallel-algorithm aggressive, which is safe now that benchmarks aren't mixed in), and any benchmark projects found are run individually afterwards, one dotnet test invocation per project, without those two flags.
  • Root cause: at least one real benchmark project (FunFair.Ethereum.DataTypes.Benchmark.Tests) rejects --long-running/--parallel-algorithm as invalid command-line arguments, producing exit code 5 ("Zero tests ran") instead of actually running. Confirmed by direct reproduction: the identical invocation without those flags passed 4/4 tests in 4m20s.
  • Also fixes the -b/--no-benchmmarks flag's broken unquoted TEST_BENCHMARKS assignment (previously a no-op), which is now load-bearing for skipping the new benchmark phase.
  • A comment points back to credfeto-global-pre-commit's src/scripts/buildtest as the source of truth for this design, so the two stay in sync.
  • Intentionally does not attempt the rest of the broader refactor already proposed on buildtest: aggressive parallel algorithm causes OOM when benchmark tests run #679 (further typo fixes, default-exclude-benchmarks semantics beyond what's above).

Closes #723
Refs #679

How Has This Been Tested

  • shellcheck --shell=sh passes on the modified script.
  • checkbashisms passes on the modified script.
  • Repo's own pre-commit hook (including changelog lint) passes on both commits.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • Unreleased section of CHANGELOG.md has been updated with details of this PR.

@dnyw4l3n13 dnyw4l3n13 added the auto-pr Pull request created automatically label Jul 16, 2026
@credfeto credfeto changed the title fix: remove --long-running/--parallel-algorithm flags from buildtest's dotnet test invocation fix: remove invalid dotnet test flags from buildtest's benchmark run Jul 16, 2026
@credfeto credfeto added the AI-Work Work for an AI Agent label Jul 16, 2026
@credfeto credfeto changed the title fix: remove invalid dotnet test flags from buildtest's benchmark run fix: split buildtest into a unit-test pass and a separate benchmark pass Jul 16, 2026
@credfeto
credfeto marked this pull request as ready for review July 16, 2026 08:56
Copilot AI review requested due to automatic review settings July 16, 2026 08:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

credfeto added 2 commits July 16, 2026 09:57
…s dotnet test invocation

Some benchmark test projects' test host rejects these as invalid
arguments (exit code 5, "Zero tests ran") instead of running,
confirmed by direct reproduction against
FunFair.Ethereum.DataTypes.Benchmark.Tests. Removing
--parallel-algorithm aggressive also addresses part of the OOM
racing problem tracked in #679.

Closes #723
Mirrors credfeto-global-pre-commit's src/scripts/buildtest design: the
main dotnet test run now always excludes every benchmark test
project (keeping --long-running/--parallel-algorithm aggressive,
safe now that benchmarks aren't mixed in), and any benchmark
projects found are run individually afterwards, one dotnet test
invocation per project, without those two flags - some benchmark
projects' test host rejects them as invalid arguments.

Also fixes the -b/--no-benchmmarks flag's broken unquoted
TEST_BENCHMARKS assignment, which is now load-bearing for skipping
the new benchmark phase.

Refs #679
@credfeto
credfeto force-pushed the fix/723-buildtest-remove-invalid-test-flags branch from 080bffe to 6e022f5 Compare July 16, 2026 08:57
@credfeto
credfeto merged commit df83968 into main Jul 16, 2026
18 of 19 checks passed
@credfeto
credfeto deleted the fix/723-buildtest-remove-invalid-test-flags branch July 16, 2026 08:57
@credfeto

Copy link
Copy Markdown
Owner

Super-linter summary

Language Validation result

All files and directories linted successfully

For more information, see the GitHub Actions workflow run

Powered by Super-linter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-Work Work for an AI Agent auto-pr Pull request created automatically

Projects

None yet

Development

Successfully merging this pull request may close these issues.

buildtest: remove --long-running/--parallel-algorithm flags that break benchmark test discovery

3 participants