BenchmarkDotNet
BenchmarkDotNet copied to clipboard
Rework "Not unique exporter for exporter type" PR
Fixes #1700
Problem
Starting with #1796 PR (#1700 issue), only one exporter per type can be.
This means that the following cases stopped working in 0.13.2:
// DefaultConfig has MarkdownExporterAttribute.Default already
// It prints a warning and Atlassian file is not created.
[MarkdownExporterAttribute.Atlassian]
public class Klass
{
[Benchmark] public void M() { }
}
Also, it means that it will never works:
// not working for ManualConfig too
[XmlExporterAttribute.Full] // it uses XmlExporter.Full under the hood
[XmlExporterAttribute.Brief] // it uses XmlExporter.Brief under the hood
Solution
- revert #1796
- compare by
IExporter.Name - rename
IExporter.Name->IExporter.IdbecauseNameis misleading, the other entities in BDN usesIdfor it:
Characteristic.Id
IAnalyser.Id
ILogger.Id
IDiagnoser.Ids
- Add
RPlotValidatorto check that user do not overridecsvseparator (Currently, all version of csv generates file with same name)
These benchmarks are manually checked:
Click me
[DryJob, MarkdownExporterAttribute.Atlassian]
public class MyTest1
{
[Benchmark] public void MyMethod() { }
}
// validator error
[CsvMeasurementsExporter(CsvSeparator.Semicolon)]
[RPlotExporter]
[DryJob]
public class MyTest2
{
[Benchmark] public void MyMethod() { }
}
[DryJob]
[MarkdownExporterAttribute.Atlassian]
[XmlExporterAttribute.Full]
[XmlExporterAttribute.Brief]
public class MyTest3
{
[Benchmark] public void MyMethod() { }
}
[DryJob]
[CustomExporter("Benchmark")]
public class MyTest4
{
[Benchmark] public void MyMethod() { }
}
public class CustomExporterAttribute : ExporterConfigBaseAttribute
{
public CustomExporterAttribute(string title) : base(new CustomExporter(title)) { }
}
public class CustomExporter : IExporter
{
private readonly string _title;
public CustomExporter(string title) => _title = title;
public string Id => GetType().Name;
public IEnumerable<string> ExportToFiles(Summary summary, ILogger consoleLogger)
{
Console.WriteLine($"{Id} - {_title}");
yield break;
}
public void ExportToLog(Summary summary, ILogger logger) { }
}
I'm not at all sure about renaming (IExporter.Name -> IExporter.Id)
I don't think anyone uses it so it one line change for customers.
We will never be able to use "C# default interface implementation" due to .NET Framework?
I am not very convinced of the solution. Because if you have developed custom exporters, no anomalies will be reported unless a validator is written.
The original issue was not written to solve the RPlotExporter problem but to prevent problems like this from happening in the future.
I am not very convinced of the solution. Because if you have developed custom exporters, no anomalies will be reported unless a validator is written.
@workgroupengineering Could you write an example please? It covers your issue (#1700)
If you mean about 'IExporterDependencies':
- only
RPlotExporteruses it. - it's internal interface.
If the plots are rewritten in C#, IExporterDependencies will be deleted.
Indeed, it would be better to remove IExporterDependencies now. And pass separator to R. But active contributors don't know R.
I encountered problem #1700 during development a PR which allowed append result to csv artifact. The artifact was overwritten.
Indeed, it would be better to remove
IExporterDependenciesnow. And passseparatortoR. But active contributors don't knowR.
Probably is better solution remove IExporterDependencies from RPlotExporter.
I would keep IExporterDependencies and would make it public it would allow you to easily extend it starting from an artifact as RPlotExporter already does.
Instead of making IExporterDependencies public, what do you think about the next one:
The naming of Json/Xml exporters is depending on "exporter style":
XmlExporter.Full -> creates "NAME-full.xml"
XmlExporter.Brief -> creates "NAME-brief.xml"
JsonExporter.Full -> creates "NAME-full.json"
JsonExporter.Brief -> creates "NAME-brief.json"
Maybe we should do the same for csv?
CsvExporter(CsvSeparator.Comma) -> creates "NAME-comma.csv"
CsvExporter(CsvSeparator.Semicolon) -> creates "NAME-semicolon.csv"
CsvExporter(CsvSeparator.CurrentCulture) -> creates "NAME-comma.csv" or creates "NAME-semicolon.csv"
CsvSeparator.CurrentCulture looks dangerous, but as I understand it, R graphs depend on this
[Edit]
CsvExporter(CsvSeparator.CurrentCulture) -> can create "NAME-currentculture.csv" 🤔
@YegorStepanov, please don't forget individual names for the Markdown exporters.