Skip to content
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

Introduce MockToolchain #2178

Merged
merged 2 commits into from
Feb 14, 2023
Merged

Introduce MockToolchain #2178

merged 2 commits into from
Feb 14, 2023

Conversation

AndreyAkinshin
Copy link
Member

This new toolchain allows writing simple unit tests for summary tables without a need to actually run benchmarks. The usage of the new API is shown in RatioColumnTest.

@AndreyAkinshin
Copy link
Member Author

@adamsitnik what do you think?

Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

Overall it LGTM, but I believe that we could simplify it.

{
public static class MockHelper
{
public static Summary Run<T>(ITestOutputHelper output)
Copy link
Member

Choose a reason for hiding this comment

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

We could make this method accept a mandatory Func<BenchmarkCase, List<Measurement>> argument, pass it to MockToolchain instance and remove the need of having a custom attribute, interface and reflection.

Sample usage could look like this:

MockHelper.Run<BenchmarkClass>(output, benchmarkCase =>
    benchmarkCase.Descriptor.WorkloadMethod.Name switch
    {
        "Foo" => CreateFromValues(new double[] { 2, 2, 2 }),
        "Bar" => CreateFromValues(new double[] { 4, 4, 4 }),
        _ => throw new InvalidOperationException()
    });

Copy link
Member Author

Choose a reason for hiding this comment

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

@adamsitnik good idea, I like it! I rewrote the implementation, could you please check out the new version?

@AndreyAkinshin
Copy link
Member Author

@adamsitnik I just merged #2268 to mocktoolchain, thanks for the edits!
Can I merge this PR now?

Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

LGTM, and great idea BTW!

@AndreyAkinshin AndreyAkinshin merged commit f76c682 into master Feb 14, 2023
@AndreyAkinshin AndreyAkinshin deleted the mocktoolchain branch February 14, 2023 18:05
@AndreyAkinshin AndreyAkinshin added this to the v0.13.5 milestone Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants