Skip to content

Commit

Permalink
Issue2394 multiple markdown exporters not possible even with differen…
Browse files Browse the repository at this point in the history
…t names (#2395)

* Merge exporters by exporter.Name instead of exporter.GetType()

* Added test that MarkdownExporter.GitHub and MarkdownExporter.Atlassian are both accepted in the merged configuration
  • Loading branch information
bstordrup committed Aug 10, 2023
1 parent 2c999b9 commit d058c7b
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 15 deletions.
28 changes: 14 additions & 14 deletions src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -109,28 +109,28 @@ void AddWarning(string message)
configAnalyse.Add(conclusion);
}

var mergeDictionary = new Dictionary<System.Type, IExporter>();
var mergeDictionary = new Dictionary<string, IExporter>();

foreach (var exporter in exporters)
{
var exporterType = exporter.GetType();
if (mergeDictionary.ContainsKey(exporterType))
var exporterName = exporter.Name;
if (mergeDictionary.ContainsKey(exporterName))
{
AddWarning($"The exporter {exporterType} is already present in configuration. There may be unexpected results.");
AddWarning($"The exporter {exporterName} is already present in configuration. There may be unexpected results.");
}
mergeDictionary[exporterType] = exporter;
mergeDictionary[exporterName] = exporter;
}


foreach (var diagnoser in uniqueDiagnosers)
foreach (var exporter in diagnoser.Exporters)
{
var exporterType = exporter.GetType();
if (mergeDictionary.ContainsKey(exporterType))
var exporterName = exporter.Name;
if (mergeDictionary.ContainsKey(exporterName))
{
AddWarning($"The exporter {exporterType} of {diagnoser.GetType().Name} is already present in configuration. There may be unexpected results.");
AddWarning($"The exporter {exporterName} of {diagnoser.GetType().Name} is already present in configuration. There may be unexpected results.");
}
mergeDictionary[exporterType] = exporter;
mergeDictionary[exporterName] = exporter;
}

var result = mergeDictionary.Values.ToList();
Expand All @@ -143,7 +143,7 @@ void AddWarning(string message)
if (hardwareCounterDiagnoser != default(IHardwareCountersDiagnoser) && disassemblyDiagnoser != default(DisassemblyDiagnoser))
result.Add(new InstructionPointerExporter(hardwareCounterDiagnoser, disassemblyDiagnoser));

for (int i = result.Count - 1; i >=0; i--)
for (int i = result.Count - 1; i >= 0; i--)
if (result[i] is IExporterDependencies exporterDependencies)
foreach (var dependency in exporterDependencies.Dependencies)
/*
Expand All @@ -165,7 +165,7 @@ void AddWarning(string message)
* "The CsvMeasurementsExporter is already present in the configuration. There may be unexpected results of RPlotExporter.
*
*/
if (!result.Any(exporter=> exporter.GetType() == dependency.GetType()))
if (!result.Any(exporter => exporter.GetType() == dependency.GetType()))
result.Insert(i, dependency); // All the exporter dependencies should be added before the exporter
else
{
Expand All @@ -186,9 +186,9 @@ private static ImmutableHashSet<IAnalyser> GetAnalysers(IEnumerable<IAnalyser> a
builder.Add(analyser);

foreach (var diagnoser in uniqueDiagnosers)
foreach (var analyser in diagnoser.Analysers)
if (!builder.Contains(analyser))
builder.Add(analyser);
foreach (var analyser in diagnoser.Analysers)
if (!builder.Contains(analyser))
builder.Add(analyser);

return builder.ToImmutable();
}
Expand Down
15 changes: 14 additions & 1 deletion tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,19 @@ public void DuplicateExportersAreExcluded()
Assert.Same(MarkdownExporter.GitHub, final.GetExporters().Single());
}

[Fact]
public void MultipleExportersOfSameTypeWithDifferentNamesAreAccepted()
{
var mutable = ManualConfig.CreateEmpty();

mutable.AddExporter(MarkdownExporter.GitHub);
mutable.AddExporter(MarkdownExporter.Atlassian);

var final = ImmutableConfigBuilder.Create(mutable);

Assert.Equal(2, final.GetExporters().Count());
}

[Fact]
public void DuplicateAnalyzersAreExcluded()
{
Expand Down Expand Up @@ -380,7 +393,7 @@ private static ImmutableConfig[] AddLeftToTheRightAndRightToTheLef(ManualConfig
var leftAddedToTheRight = ManualConfig.Create(right);
leftAddedToTheRight.Add(left);

return new[]{ rightAddedToLeft.CreateImmutableConfig(), leftAddedToTheRight.CreateImmutableConfig() };
return new[] { rightAddedToLeft.CreateImmutableConfig(), leftAddedToTheRight.CreateImmutableConfig() };
}

public class TestExporter : IExporter, IExporterDependencies
Expand Down

0 comments on commit d058c7b

Please sign in to comment.