Skip to content

Collect declaration errors - #3268

Open
timcassell wants to merge 1 commit into
masterfrom
feature/collect-declaration-errors
Open

timcassell wants to merge 1 commit into
masterfrom
feature/collect-declaration-errors

Conversation

@timcassell

Copy link
Copy Markdown
Collaborator

Core changes:

  1. Discovery collects declaration errors instead of throwing.
    • BenchmarkRunInfo gains DeclarationErrors, and BenchmarkRunnerClean merges them to validation errors.

Behavior changes:

  1. A type keeps the cases it could build. One bad member costs only the cases it would have produced, which is what lets --list and the test adapter enumerate a type that has one.
  2. Three places showed something other than the error, and no longer do. Each one had a type left with no cases look like a type that never declared any.
    • BenchmarkRunnerClean said No [Benchmark] attribute found on 'X' — what it says about a type with no benchmarks at all. It now says why the method was refused.
    • TypeFilter dropped such a type before the runner saw it, so from the command line the error vanished and the run said the filter matched nothing. It now keeps a type that has something to report.
    • The message arrived as the summary's title, since the throw became Summary.ValidationFailed(e.Message, ...). It is now one of Summary.ValidationErrors, and the title is a title.
  3. A source that is both unreadable and declared wrong now reports both. Discovery says it could not read the values, and SourceReturnTypeValidator says the declared return type is not one it could have read from. The throw used to pre-empt the second, which is the more useful of the two.

Bug fixes:

  1. A source row that is not the shape its declaration promised is reported rather than crashing.
    • Values are read through the non-generic IEnumerable, which a type is free to implement differently from the IEnumerable<T> it declares. A row could therefore arrive that was not the declared ValueTuple, and the walk to its items failed reaching for a field with a message blaming BenchmarkDotNet. Both readings now check the row before reaching into it and name what arrived instead.
  2. BenchmarkDotNet.TestAdapter no longer loses these errors. All three of its reshapes rebuilt a BenchmarkRunInfo through the constructor, which silently dropped whatever the caller did not name — so under dotnet test a type with a bad declaration would run its good benchmarks and report nothing.

Breaking changes:

  1. BenchmarkRunInfo sealed and public constructors removed.
    • WithBenchmarks and WithConfig added to modify it instead.
  2. InvalidBenchmarkDeclarationException removed.

Other Changes:

  1. BenchmarkRunInfo and Descriptor validate the types of their inputs.
  2. Helpers.Assertion removed in favour of ArgumentNullException.ThrowIfNull.
  3. New and updated tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant