Skip to content

Allow ArgumentsSource to reference methods in other types #2748

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 6, 2025

Conversation

JimmyCushnie
Copy link
Contributor

Implements #2744

I added a new constructor for ArgumentsSourceAttribute which takes a custom type. I implemented the logic in BenchmarkConverter so that the custom type can be used. The behavior of the existing constructor is preserved.

I updated the docs to explain the new feature.

I renamed a few internal method parameters for clarity.

This is my first contribution to BenchmarkDotNet. Please let me know if I did anything wrong!

@timcassell
Copy link
Collaborator

Maybe do the same thing for ParamsSource?

@JimmyCushnie
Copy link
Contributor Author

Makes sense. Pushed!

@timcassell
Copy link
Collaborator

Can you also please add or update integration tests?

@JimmyCushnie
Copy link
Contributor Author

I'm having trouble grokking the tests in this repo. I could use some help. Where would be the appropriate place for the new/modified tests? I can't find any existing tests that involve ArgumentsSource or ParamsSource. The tests for other attributes seem unrelated.

@JimmyCushnie
Copy link
Contributor Author

JimmyCushnie commented Jun 4, 2025

Thanks very much. Added two new tests to verify the newly-added behavior. All tests are passing locally.

Edit: and I see they're also passing on the CI, nice

Copy link
Collaborator

@timcassell timcassell left a comment

Choose a reason for hiding this comment

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

Thanks @JimmyCushnie!

@timcassell timcassell merged commit ba1f998 into dotnet:master Jun 6, 2025
8 checks passed
@timcassell timcassell added this to the v0.15.1 milestone Jun 6, 2025
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.

2 participants