From fb0181fcbdcc25ce690010a5b5d0cab7ca182b07 Mon Sep 17 00:00:00 2001 From: Justin Perez Date: Tue, 11 Apr 2023 15:43:50 -0700 Subject: [PATCH 01/15] add experiment configs --- .../Experiments/Configs/NewNugetExperiment.cs | 22 +++++++++++++ .../Experiments/IExperimentConfiguration.cs | 31 +++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/NewNugetExperiment.cs create mode 100644 src/Microsoft.ComponentDetection.Orchestrator/Experiments/IExperimentConfiguration.cs diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/NewNugetExperiment.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/NewNugetExperiment.cs new file mode 100644 index 000000000..44a085370 --- /dev/null +++ b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/NewNugetExperiment.cs @@ -0,0 +1,22 @@ +namespace Microsoft.ComponentDetection.Orchestrator.Experiments.Configs; + +using Microsoft.ComponentDetection.Common.Experiments; +using Microsoft.ComponentDetection.Contracts; +using Microsoft.ComponentDetection.Detectors.NuGet; + +/// +/// Comparing the new NuGet detector approach to the old one. +/// +public class NewNugetExperiment : IExperimentConfiguration +{ + /// + public string Name => "NewNugetDetector"; + + /// + public bool IsInControlGroup(IComponentDetector componentDetector) => + componentDetector is NuGetComponentDetector or NuGetProjectModelProjectCentricComponentDetector; + + /// + public bool IsInExperimentGroup(IComponentDetector componentDetector) => + componentDetector is NuGetComponentDetector or NuGetPackagesConfigDetector; +} diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/IExperimentConfiguration.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/IExperimentConfiguration.cs new file mode 100644 index 000000000..da3b203d8 --- /dev/null +++ b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/IExperimentConfiguration.cs @@ -0,0 +1,31 @@ +namespace Microsoft.ComponentDetection.Common.Experiments; + +using Microsoft.ComponentDetection.Contracts; + +/// +/// Defines the configuration for an experiment. An experiment is a set of detectors that are grouped into control and +/// experiment groups. The control group is used to determine the baseline for the experiment. The experiment group is +/// used to determine the impact of the experiment on the baseline. The unique set of components from the two sets of +/// detectors is compared and differences are reported via telemetry. +/// +public interface IExperimentConfiguration +{ + /// + /// The name of the experiment. + /// + string Name { get; } + + /// + /// Specifies if the detector is in the control group. + /// + /// The detector. + /// true if the detector is in the control group; otherwise, false. + bool IsInControlGroup(IComponentDetector componentDetector); + + /// + /// Specifies if the detector is in the control group. + /// + /// The detector. + /// true if the detector is in the experiment group; otherwise, false. + bool IsInExperimentGroup(IComponentDetector componentDetector); +} From cdf322f26abb74e9de130b6ec229bb17952abd35 Mon Sep 17 00:00:00 2001 From: Justin Perez Date: Wed, 12 Apr 2023 16:12:39 -0700 Subject: [PATCH 02/15] feat: add detector experiments --- .../{ => Configs}/IExperimentConfiguration.cs | 2 +- .../Experiments/Configs/NewNugetExperiment.cs | 3 +- .../Experiments/DefaultExperimentProcessor.cs | 36 ++++++ .../Experiments/ExperimentService.cs | 83 ++++++++++++ .../Experiments/IExperimentProcessor.cs | 19 +++ .../Experiments/IExperimentService.cs | 24 ++++ .../Experiments/Models/Experiment.cs | 50 ++++++++ .../Experiments/Models/ExperimentComponent.cs | 37 ++++++ .../Experiments/Models/ExperimentDiff.cs | 119 ++++++++++++++++++ .../Extensions/ServiceCollectionExtensions.cs | 7 ++ .../Services/DetectorProcessingService.cs | 8 ++ 11 files changed, 385 insertions(+), 3 deletions(-) rename src/Microsoft.ComponentDetection.Orchestrator/Experiments/{ => Configs}/IExperimentConfiguration.cs (94%) create mode 100644 src/Microsoft.ComponentDetection.Orchestrator/Experiments/DefaultExperimentProcessor.cs create mode 100644 src/Microsoft.ComponentDetection.Orchestrator/Experiments/ExperimentService.cs create mode 100644 src/Microsoft.ComponentDetection.Orchestrator/Experiments/IExperimentProcessor.cs create mode 100644 src/Microsoft.ComponentDetection.Orchestrator/Experiments/IExperimentService.cs create mode 100644 src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/Experiment.cs create mode 100644 src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/ExperimentComponent.cs create mode 100644 src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/ExperimentDiff.cs diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/IExperimentConfiguration.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/IExperimentConfiguration.cs similarity index 94% rename from src/Microsoft.ComponentDetection.Orchestrator/Experiments/IExperimentConfiguration.cs rename to src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/IExperimentConfiguration.cs index da3b203d8..ff328e440 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/IExperimentConfiguration.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/IExperimentConfiguration.cs @@ -1,4 +1,4 @@ -namespace Microsoft.ComponentDetection.Common.Experiments; +namespace Microsoft.ComponentDetection.Orchestrator.Experiments.Configs; using Microsoft.ComponentDetection.Contracts; diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/NewNugetExperiment.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/NewNugetExperiment.cs index 44a085370..65c57f006 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/NewNugetExperiment.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/NewNugetExperiment.cs @@ -1,6 +1,5 @@ namespace Microsoft.ComponentDetection.Orchestrator.Experiments.Configs; -using Microsoft.ComponentDetection.Common.Experiments; using Microsoft.ComponentDetection.Contracts; using Microsoft.ComponentDetection.Detectors.NuGet; @@ -18,5 +17,5 @@ public bool IsInControlGroup(IComponentDetector componentDetector) => /// public bool IsInExperimentGroup(IComponentDetector componentDetector) => - componentDetector is NuGetComponentDetector or NuGetPackagesConfigDetector; + componentDetector is NuGetProjectModelProjectCentricComponentDetector or NuGetPackagesConfigDetector; } diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/DefaultExperimentProcessor.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/DefaultExperimentProcessor.cs new file mode 100644 index 000000000..01d7f4ea6 --- /dev/null +++ b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/DefaultExperimentProcessor.cs @@ -0,0 +1,36 @@ +namespace Microsoft.ComponentDetection.Orchestrator.Experiments; + +using System; +using System.IO; +using System.Text.Json; +using System.Threading.Tasks; +using Microsoft.ComponentDetection.Orchestrator.Experiments.Configs; +using Microsoft.ComponentDetection.Orchestrator.Experiments.Models; +using Microsoft.Extensions.Logging; + +/// +/// The default experiment processor. Writes a JSON output file to a temporary directory. +/// +public class DefaultExperimentProcessor : IExperimentProcessor +{ + private readonly ILogger logger; + + /// + /// Initializes a new instance of the class. + /// + /// The logger. + public DefaultExperimentProcessor(ILogger logger) => this.logger = logger; + + /// + public async Task ProcessExperimentAsync(IExperimentConfiguration config, ExperimentDiff diff) + { + var filename = Path.Combine( + Path.GetTempPath(), + $"Experiment_{config.Name}_{DateTime.Now:yyyyMMddHHmmssfff}_{Environment.ProcessId}.json"); + + this.logger.LogInformation("Writing experiment {Name} results to {Filename}", config.Name, filename); + + await using var file = File.Create(filename); + await JsonSerializer.SerializeAsync(file, diff, new JsonSerializerOptions { WriteIndented = true }); + } +} diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/ExperimentService.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/ExperimentService.cs new file mode 100644 index 000000000..7bb32fe2b --- /dev/null +++ b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/ExperimentService.cs @@ -0,0 +1,83 @@ +namespace Microsoft.ComponentDetection.Orchestrator.Experiments; + +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.ComponentDetection.Contracts; +using Microsoft.ComponentDetection.Orchestrator.Experiments.Configs; +using Microsoft.ComponentDetection.Orchestrator.Experiments.Models; +using Microsoft.Extensions.Logging; + +/// +public class ExperimentService : IExperimentService +{ + private readonly List<(IExperimentConfiguration Config, Experiment Experiment)> experiments; + private readonly IEnumerable experimentProcessors; + private readonly ILogger logger; + + /// + /// Initializes a new instance of the class. + /// + /// The experiment configurations. + /// The experiment processors. + /// The logger. + public ExperimentService( + IEnumerable configs, + IEnumerable experimentProcessors, + ILogger logger) + { + this.experiments = configs.Select(x => (x, new Experiment())).ToList(); + this.experimentProcessors = experimentProcessors; + this.logger = logger; + } + + /// + public void RecordDetectorRun(IComponentDetector detector, IEnumerable components) + { + foreach (var (config, experiment) in this.experiments) + { + if (config.IsInControlGroup(detector)) + { + experiment.AddComponentsToControlGroup(components); + this.logger.LogDebug( + "Adding {Count} Components from {Id} to Control Group for {Experiment}", + components.Count(), + detector.Id, + config.Name); + } + + if (config.IsInExperimentGroup(detector)) + { + experiment.AddComponentsToExperimentalGroup(components); + this.logger.LogDebug( + "Adding {Count} Components from {Id} to Experiment Group for {Experiment}", + components.Count(), + detector.Id, + config.Name); + } + } + } + + /// + public async Task FinishAsync() + { + foreach (var (config, experiment) in this.experiments) + { + var oldComponents = experiment.ControlGroupComponents; + var newComponents = experiment.ExperimentGroupComponents; + + this.logger.LogInformation( + "Experiment {Experiment} finished and has {Count} components in the control group and {Count} components in the experiment group.", + config.Name, + oldComponents.Count, + newComponents.Count); + + var diff = new ExperimentDiff(experiment.ControlGroupComponents, experiment.ExperimentGroupComponents); + + foreach (var processor in this.experimentProcessors) + { + await processor.ProcessExperimentAsync(config, diff); + } + } + } +} diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/IExperimentProcessor.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/IExperimentProcessor.cs new file mode 100644 index 000000000..0762affc5 --- /dev/null +++ b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/IExperimentProcessor.cs @@ -0,0 +1,19 @@ +namespace Microsoft.ComponentDetection.Orchestrator.Experiments; + +using System.Threading.Tasks; +using Microsoft.ComponentDetection.Orchestrator.Experiments.Configs; +using Microsoft.ComponentDetection.Orchestrator.Experiments.Models; + +/// +/// Processes the results of an experiment. Used to report the results of an experiment, such as by writing to a file. +/// +public interface IExperimentProcessor +{ + /// + /// Asynchronously processes the results of an experiment. + /// + /// The experiment configuration. + /// The difference in components between two sets of detectors. + /// A representing the asynchronous operation. + Task ProcessExperimentAsync(IExperimentConfiguration config, ExperimentDiff diff); +} diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/IExperimentService.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/IExperimentService.cs new file mode 100644 index 000000000..abd35eed4 --- /dev/null +++ b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/IExperimentService.cs @@ -0,0 +1,24 @@ +namespace Microsoft.ComponentDetection.Orchestrator.Experiments; + +using System.Collections.Generic; +using System.Threading.Tasks; +using Microsoft.ComponentDetection.Contracts; + +/// +/// Service for recording detector results and processing the results for any active experiments. +/// +public interface IExperimentService +{ + /// + /// Records the results of a detector execution and processes the results for any active experiments. + /// + /// The detector. + /// The detected components from the . + void RecordDetectorRun(IComponentDetector detector, IEnumerable components); + + /// + /// Called when all detectors have finished executing. Processes the experiments and reports the results. + /// + /// A representing the asynchronous operation. + Task FinishAsync(); +} diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/Experiment.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/Experiment.cs new file mode 100644 index 000000000..c41067e7c --- /dev/null +++ b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/Experiment.cs @@ -0,0 +1,50 @@ +namespace Microsoft.ComponentDetection.Orchestrator.Experiments.Models; + +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using Microsoft.ComponentDetection.Contracts; + +/// +/// Stores the results of a detector execution for an experiment. Buckets components into a control group and an experimental group. +/// +public class Experiment +{ + private readonly HashSet controlGroupComponents = new(); + + private readonly HashSet experimentGroupComponents = new(); + + /// + /// The set of components in the control group. + /// + public IImmutableSet ControlGroupComponents => + this.controlGroupComponents.ToImmutableHashSet(); + + /// + /// The set of components in the experimental group. + /// + public IImmutableSet ExperimentGroupComponents => + this.experimentGroupComponents.ToImmutableHashSet(); + + /// + /// Adds the components to the control group. + /// + /// The components. + public void AddComponentsToControlGroup(IEnumerable components) => + AddComponents(this.controlGroupComponents, components); + + /// + /// Adds the components to the experimental group. + /// + /// The components. + public void AddComponentsToExperimentalGroup(IEnumerable components) => + AddComponents(this.experimentGroupComponents, components); + + private static void AddComponents(ISet group, IEnumerable components) + { + foreach (var experimentComponent in components.Select(x => new ExperimentComponent(x))) + { + group.Add(experimentComponent); + } + } +} diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/ExperimentComponent.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/ExperimentComponent.cs new file mode 100644 index 000000000..850beeaea --- /dev/null +++ b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/ExperimentComponent.cs @@ -0,0 +1,37 @@ +namespace Microsoft.ComponentDetection.Orchestrator.Experiments.Models; + +using System.Collections.Generic; +using System.Linq; +using Microsoft.ComponentDetection.Contracts; + +/// +/// A model representing a component detected by a detector, as relevant to an experiment. +/// +public record ExperimentComponent +{ + /// + /// Creates a new from a . + /// + /// The detected component. + public ExperimentComponent(DetectedComponent detectedComponent) + { + this.Id = detectedComponent.Component.Id; + this.DevelopmentDependency = detectedComponent.DevelopmentDependency ?? false; + this.RootIds = detectedComponent.DependencyRoots?.Select(x => x.Id).ToHashSet() ?? new HashSet(); + } + + /// + /// The component ID. + /// + public string Id { get; } + + /// + /// true if the component is a development dependency; otherwise, false. + /// + public bool DevelopmentDependency { get; } + + /// + /// The set of root component IDs for this component. + /// + public HashSet RootIds { get; } +} diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/ExperimentDiff.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/ExperimentDiff.cs new file mode 100644 index 000000000..267e1635d --- /dev/null +++ b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/ExperimentDiff.cs @@ -0,0 +1,119 @@ +namespace Microsoft.ComponentDetection.Orchestrator.Experiments.Models; + +using System.Collections.Generic; +using System.Linq; + +/// +/// A model for the difference between two sets of instances. +/// +public class ExperimentDiff +{ + /// + /// Creates a new . + /// + /// A set of components from the control group. + /// A set of components from the experimental group. + public ExperimentDiff( + IEnumerable oldComponents, + IEnumerable newComponents) + { + var oldComponentDictionary = oldComponents.ToDictionary(x => x.Id); + var newComponentDictionary = newComponents.ToDictionary(x => x.Id); + + this.AddedIds = newComponentDictionary.Keys.Except(oldComponentDictionary.Keys).ToList(); + this.RemovedIds = oldComponentDictionary.Keys.Except(newComponentDictionary.Keys).ToList(); + + this.DevelopmentDependencyChanges = new List(); + this.AddedRootIds = new Dictionary>(); + this.RemovedRootIds = new Dictionary>(); + + // Need performance benchmark to see if this is worth parallelization + foreach (var id in newComponentDictionary.Keys.Intersect(oldComponentDictionary.Keys)) + { + var oldComponent = oldComponentDictionary[id]; + var newComponent = newComponentDictionary[id]; + + if (oldComponent.DevelopmentDependency != newComponent.DevelopmentDependency) + { + this.DevelopmentDependencyChanges.Add(new DevelopmentDependencyChange( + id, + oldComponent.DevelopmentDependency, + newComponent.DevelopmentDependency)); + } + + var addedRootIds = newComponent.RootIds.Except(oldComponent.RootIds).ToHashSet(); + var removedRootIds = oldComponent.RootIds.Except(newComponent.RootIds).ToHashSet(); + + if (addedRootIds.Count > 0) + { + this.AddedRootIds[id] = addedRootIds; + } + + if (removedRootIds.Count > 0) + { + this.RemovedRootIds[id] = removedRootIds; + } + } + } + + /// + /// Gets a list of component IDs that were present in the experimental group but not the control group. + /// + public List AddedIds { get; } + + /// + /// Gets a list of component IDs that were present in the control group but not the experimental group. + /// + public List RemovedIds { get; } + + /// + /// Gets a list of changes to the development dependency status of components. + /// + public List DevelopmentDependencyChanges { get; } + + /// + /// Gets a dictionary of component IDs to the set of root IDs that were added to the component. The component ID + /// is the key. + /// + public Dictionary> AddedRootIds { get; } + + /// + /// Gets a dictionary of component IDs to the set of root IDs that were removed from the component. The component + /// ID is the key. + /// + public Dictionary> RemovedRootIds { get; } + + /// + /// Stores information about a change to the development dependency status of a component. + /// + public class DevelopmentDependencyChange + { + /// + /// Creates a new . + /// + /// The component ID. + /// The old value of the development dependency status. + /// The new value of the development dependency status. + public DevelopmentDependencyChange(string id, bool oldValue, bool newValue) + { + this.Id = id; + this.OldValue = oldValue; + this.NewValue = newValue; + } + + /// + /// Gets the component ID. + /// + public string Id { get; } + + /// + /// Gets the old value of the development dependency status. + /// + public bool OldValue { get; } + + /// + /// Gets the new value of the development dependency status. + /// + public bool NewValue { get; } + } +} diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Extensions/ServiceCollectionExtensions.cs b/src/Microsoft.ComponentDetection.Orchestrator/Extensions/ServiceCollectionExtensions.cs index f031e8866..c3d4be9ee 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/Extensions/ServiceCollectionExtensions.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/Extensions/ServiceCollectionExtensions.cs @@ -22,6 +22,8 @@ using Microsoft.ComponentDetection.Detectors.Yarn; using Microsoft.ComponentDetection.Detectors.Yarn.Parsers; using Microsoft.ComponentDetection.Orchestrator.ArgumentSets; +using Microsoft.ComponentDetection.Orchestrator.Experiments; +using Microsoft.ComponentDetection.Orchestrator.Experiments.Configs; using Microsoft.ComponentDetection.Orchestrator.Services; using Microsoft.ComponentDetection.Orchestrator.Services.GraphTranslation; using Microsoft.Extensions.DependencyInjection; @@ -65,6 +67,11 @@ public static IServiceCollection AddComponentDetection(this IServiceCollection s services.AddSingleton(); services.AddSingleton(); + // Experiments + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + // Detectors // CocoaPods services.AddSingleton(); diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs b/src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs index c32d5585b..0b241b01b 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs @@ -1,4 +1,5 @@ namespace Microsoft.ComponentDetection.Orchestrator.Services; + using System; using System.Collections.Concurrent; using System.Collections.Generic; @@ -14,6 +15,7 @@ using Microsoft.ComponentDetection.Contracts; using Microsoft.ComponentDetection.Contracts.BcdeModels; using Microsoft.ComponentDetection.Orchestrator.ArgumentSets; +using Microsoft.ComponentDetection.Orchestrator.Experiments; using Microsoft.Extensions.Logging; using Newtonsoft.Json; using static System.Environment; @@ -22,12 +24,15 @@ public class DetectorProcessingService : IDetectorProcessingService { private readonly IObservableDirectoryWalkerFactory scanner; private readonly ILogger logger; + private readonly IExperimentService experimentService; public DetectorProcessingService( IObservableDirectoryWalkerFactory scanner, + IExperimentService experimentService, ILogger logger) { this.scanner = scanner; + this.experimentService = experimentService; this.logger = logger; } @@ -104,6 +109,8 @@ public async Task ProcessDetectorsAsync(IDetectionArgu exitCode = resultCode; } + this.experimentService.RecordDetectorRun(detector, detectedComponents); + if (isExperimentalDetector) { return (new IndividualDetectorScanResult(), new ComponentRecorder(), detector); @@ -115,6 +122,7 @@ public async Task ProcessDetectorsAsync(IDetectionArgu }).ToList(); var results = await Task.WhenAll(scanTasks); + await this.experimentService.FinishAsync(); var detectorProcessingResult = this.ConvertDetectorResultsIntoResult(results, exitCode); From d065819d1b2a980a1f0904d772763e2b9b38bed2 Mon Sep 17 00:00:00 2001 From: Justin Perez Date: Thu, 13 Apr 2023 08:36:02 -0700 Subject: [PATCH 03/15] test: `ExperimentService` --- .../Experiments/ExperimentServiceTests.cs | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentServiceTests.cs diff --git a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentServiceTests.cs b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentServiceTests.cs new file mode 100644 index 000000000..e6b40da43 --- /dev/null +++ b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentServiceTests.cs @@ -0,0 +1,80 @@ +namespace Microsoft.ComponentDetection.Orchestrator.Tests.Experiments; + +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.ComponentDetection.Contracts; +using Microsoft.ComponentDetection.Contracts.TypedComponent; +using Microsoft.ComponentDetection.Orchestrator.Experiments; +using Microsoft.ComponentDetection.Orchestrator.Experiments.Configs; +using Microsoft.ComponentDetection.Orchestrator.Experiments.Models; +using Microsoft.Extensions.Logging; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; + +[TestClass] +[TestCategory("Governance/All")] +[TestCategory("Governance/ComponentDetection")] +public class ExperimentServiceTests +{ + private readonly Mock experimentConfigMock; + private readonly Mock experimentProcessorMock; + private readonly Mock> loggerMock; + private readonly Mock detectorMock; + + public ExperimentServiceTests() + { + this.experimentConfigMock = new Mock(); + this.experimentProcessorMock = new Mock(); + this.loggerMock = new Mock>(); + this.detectorMock = new Mock(); + } + + [TestMethod] + public void RecordDetectorRun_AddsComponentsToControlAndExperimentGroup() + { + var components = new List + { + new(new NpmComponent("ComponentA", "1.0.0")), + new(new NpmComponent("ComponentB", "1.0.0")), + }; + + this.experimentConfigMock.Setup(x => x.IsInControlGroup(this.detectorMock.Object)).Returns(true); + this.experimentConfigMock.Setup(x => x.IsInExperimentGroup(this.detectorMock.Object)).Returns(true); + + var service = new ExperimentService( + new[] { this.experimentConfigMock.Object }, + Enumerable.Empty(), + this.loggerMock.Object); + + service.RecordDetectorRun(this.detectorMock.Object, components); + + this.experimentConfigMock.Verify(x => x.IsInControlGroup(this.detectorMock.Object), Times.Once()); + this.experimentConfigMock.Verify(x => x.IsInExperimentGroup(this.detectorMock.Object), Times.Once()); + } + + [TestMethod] + public async Task FinishAsync_ProcessesExperimentsAsync() + { + var components = new List + { + new(new NpmComponent("ComponentA", "1.0.0")), + new(new NpmComponent("ComponentB", "1.0.0")), + }; + + this.experimentConfigMock.Setup(x => x.IsInControlGroup(this.detectorMock.Object)).Returns(true); + this.experimentConfigMock.Setup(x => x.IsInExperimentGroup(this.detectorMock.Object)).Returns(true); + + var service = new ExperimentService( + new[] { this.experimentConfigMock.Object }, + new[] { this.experimentProcessorMock.Object }, + this.loggerMock.Object); + service.RecordDetectorRun(this.detectorMock.Object, components); + + await service.FinishAsync(); + + this.experimentProcessorMock.Verify( + x => x.ProcessExperimentAsync(this.experimentConfigMock.Object, It.IsAny()), + Times.Once()); + } +} From 43a4bdc435fd7e64402ecc57cd45abe99d2e9659 Mon Sep 17 00:00:00 2001 From: Justin Perez Date: Thu, 13 Apr 2023 08:36:44 -0700 Subject: [PATCH 04/15] test: `DetectorProcessingService` does experiments --- .../DetectorProcessingServiceTests.cs | 49 ++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DetectorProcessingServiceTests.cs b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DetectorProcessingServiceTests.cs index 675e339f8..7c45eea63 100644 --- a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DetectorProcessingServiceTests.cs +++ b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DetectorProcessingServiceTests.cs @@ -1,4 +1,5 @@ namespace Microsoft.ComponentDetection.Orchestrator.Tests.Services; + using System; using System.Collections.Generic; using System.IO; @@ -10,6 +11,7 @@ namespace Microsoft.ComponentDetection.Orchestrator.Tests.Services; using Microsoft.ComponentDetection.Contracts; using Microsoft.ComponentDetection.Contracts.TypedComponent; using Microsoft.ComponentDetection.Orchestrator.ArgumentSets; +using Microsoft.ComponentDetection.Orchestrator.Experiments; using Microsoft.ComponentDetection.Orchestrator.Services; using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -36,6 +38,7 @@ public class DetectorProcessingServiceTests private readonly Mock> loggerMock; private readonly DetectorProcessingService serviceUnderTest; private readonly Mock directoryWalkerFactory; + private readonly Mock experimentServiceMock; private readonly Mock firstFileComponentDetectorMock; private readonly Mock secondFileComponentDetectorMock; @@ -49,10 +52,11 @@ public class DetectorProcessingServiceTests public DetectorProcessingServiceTests() { + this.experimentServiceMock = new Mock(); this.loggerMock = new Mock>(); this.directoryWalkerFactory = new Mock(); this.serviceUnderTest = - new DetectorProcessingService(this.directoryWalkerFactory.Object, this.loggerMock.Object); + new DetectorProcessingService(this.directoryWalkerFactory.Object, this.experimentServiceMock.Object, this.loggerMock.Object); this.firstFileComponentDetectorMock = this.SetupFileDetectorMock("firstFileDetectorId"); this.secondFileComponentDetectorMock = this.SetupFileDetectorMock("secondFileDetectorId"); @@ -505,6 +509,49 @@ public async Task ProcessDetectorsAsync_HandlesDetectorArgsAsync() .And.Contain("arg3", "val3"); } + [TestMethod] + public async Task ProcessDetectorsAsync_FinishesExperimentsAsync() + { + var args = DefaultArgs; + + this.detectorsToUse = new[] + { + this.firstFileComponentDetectorMock.Object, this.secondFileComponentDetectorMock.Object, + }; + + await this.serviceUnderTest.ProcessDetectorsAsync(args, this.detectorsToUse, new DetectorRestrictions()); + + this.experimentServiceMock.Verify(x => x.FinishAsync(), Times.Once()); + } + + [TestMethod] + public async Task ProcessDetectorsAsync_RecordsDetectorRunsAsync() + { + this.detectorsToUse = new[] + { + this.firstFileComponentDetectorMock.Object, this.secondFileComponentDetectorMock.Object, + }; + + var firstComponents = new[] { this.componentDictionary[this.firstFileComponentDetectorMock.Object.Id] }; + var secondComponents = new[] { this.componentDictionary[this.secondFileComponentDetectorMock.Object.Id] }; + + await this.serviceUnderTest.ProcessDetectorsAsync(DefaultArgs, this.detectorsToUse, new DetectorRestrictions()); + + this.experimentServiceMock.Verify( + x => + x.RecordDetectorRun( + It.Is(detector => detector == this.firstFileComponentDetectorMock.Object), + It.Is>(components => components.SequenceEqual(firstComponents))), + Times.Once()); + + this.experimentServiceMock.Verify( + x => + x.RecordDetectorRun( + It.Is(detector => detector == this.secondFileComponentDetectorMock.Object), + It.Is>(components => components.SequenceEqual(secondComponents))), + Times.Once()); + } + private Mock SetupFileDetectorMock(string id) { var mockFileDetector = new Mock(); From cf4a07b07893fa388636484acc49bae322c9eab9 Mon Sep 17 00:00:00 2001 From: Justin Perez Date: Thu, 13 Apr 2023 08:44:22 -0700 Subject: [PATCH 05/15] test: `DetectorProcessingService` does experiments --- .../Services/DetectorProcessingServiceTests.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DetectorProcessingServiceTests.cs b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DetectorProcessingServiceTests.cs index 7c45eea63..37eb40229 100644 --- a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DetectorProcessingServiceTests.cs +++ b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DetectorProcessingServiceTests.cs @@ -512,14 +512,12 @@ public async Task ProcessDetectorsAsync_HandlesDetectorArgsAsync() [TestMethod] public async Task ProcessDetectorsAsync_FinishesExperimentsAsync() { - var args = DefaultArgs; - this.detectorsToUse = new[] { this.firstFileComponentDetectorMock.Object, this.secondFileComponentDetectorMock.Object, }; - await this.serviceUnderTest.ProcessDetectorsAsync(args, this.detectorsToUse, new DetectorRestrictions()); + await this.serviceUnderTest.ProcessDetectorsAsync(DefaultArgs, this.detectorsToUse, new DetectorRestrictions()); this.experimentServiceMock.Verify(x => x.FinishAsync(), Times.Once()); } From d8ddcd805d659f1665196256f63f4c43660723ee Mon Sep 17 00:00:00 2001 From: Justin Perez Date: Thu, 13 Apr 2023 10:24:05 -0700 Subject: [PATCH 06/15] refactor: use `FileWritingService` --- .../FileWritingService.cs | 7 +++++++ .../IFileWritingService.cs | 3 +++ .../Experiments/DefaultExperimentProcessor.cs | 20 +++++++++++-------- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Common/FileWritingService.cs b/src/Microsoft.ComponentDetection.Common/FileWritingService.cs index d24ee2002..9539bb9e6 100644 --- a/src/Microsoft.ComponentDetection.Common/FileWritingService.cs +++ b/src/Microsoft.ComponentDetection.Common/FileWritingService.cs @@ -48,6 +48,13 @@ public void WriteFile(string relativeFilePath, string text) } } + public async Task WriteFileAsync(string relativeFilePath, string text) + { + relativeFilePath = this.ResolveFilePath(relativeFilePath); + + await File.WriteAllTextAsync(relativeFilePath, text); + } + public void WriteFile(FileInfo relativeFilePath, string text) { File.WriteAllText(relativeFilePath.FullName, text); diff --git a/src/Microsoft.ComponentDetection.Common/IFileWritingService.cs b/src/Microsoft.ComponentDetection.Common/IFileWritingService.cs index e9a9341b4..8ef32b030 100644 --- a/src/Microsoft.ComponentDetection.Common/IFileWritingService.cs +++ b/src/Microsoft.ComponentDetection.Common/IFileWritingService.cs @@ -2,6 +2,7 @@ using System; using System.IO; +using System.Threading.Tasks; // All file paths are relative and will replace occurrences of {timestamp} with the shared file timestamp. public interface IFileWritingService : IDisposable, IAsyncDisposable @@ -12,6 +13,8 @@ public interface IFileWritingService : IDisposable, IAsyncDisposable void WriteFile(string relativeFilePath, string text); + Task WriteFileAsync(string relativeFilePath, string text); + void WriteFile(FileInfo relativeFilePath, string text); string ResolveFilePath(string relativeFilePath); diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/DefaultExperimentProcessor.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/DefaultExperimentProcessor.cs index 01d7f4ea6..dbe47107f 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/DefaultExperimentProcessor.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/DefaultExperimentProcessor.cs @@ -1,9 +1,9 @@ namespace Microsoft.ComponentDetection.Orchestrator.Experiments; using System; -using System.IO; using System.Text.Json; using System.Threading.Tasks; +using Microsoft.ComponentDetection.Common; using Microsoft.ComponentDetection.Orchestrator.Experiments.Configs; using Microsoft.ComponentDetection.Orchestrator.Experiments.Models; using Microsoft.Extensions.Logging; @@ -13,24 +13,28 @@ /// public class DefaultExperimentProcessor : IExperimentProcessor { + private readonly IFileWritingService fileWritingService; private readonly ILogger logger; /// /// Initializes a new instance of the class. /// + /// The file writing service. /// The logger. - public DefaultExperimentProcessor(ILogger logger) => this.logger = logger; + public DefaultExperimentProcessor(IFileWritingService fileWritingService, ILogger logger) + { + this.fileWritingService = fileWritingService; + this.logger = logger; + } /// public async Task ProcessExperimentAsync(IExperimentConfiguration config, ExperimentDiff diff) { - var filename = Path.Combine( - Path.GetTempPath(), - $"Experiment_{config.Name}_{DateTime.Now:yyyyMMddHHmmssfff}_{Environment.ProcessId}.json"); + var filename = $"Experiment_{config.Name}_{{timestamp}}_{Environment.ProcessId}.json"; - this.logger.LogInformation("Writing experiment {Name} results to {Filename}", config.Name, filename); + this.logger.LogInformation("Writing experiment {Name} results to {Filename}", config.Name, this.fileWritingService.ResolveFilePath(filename)); - await using var file = File.Create(filename); - await JsonSerializer.SerializeAsync(file, diff, new JsonSerializerOptions { WriteIndented = true }); + var serializedDiff = JsonSerializer.Serialize(diff, new JsonSerializerOptions { WriteIndented = true }); + await this.fileWritingService.WriteFileAsync(filename, serializedDiff); } } From 79b755b6e424babde8a394923d6989554acc488f Mon Sep 17 00:00:00 2001 From: Justin Perez Date: Thu, 13 Apr 2023 10:24:23 -0700 Subject: [PATCH 07/15] test: add experiment test utils --- .../Experiments/ExperimentTestUtils.cs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentTestUtils.cs diff --git a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentTestUtils.cs b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentTestUtils.cs new file mode 100644 index 000000000..eaf29ed3d --- /dev/null +++ b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentTestUtils.cs @@ -0,0 +1,23 @@ +namespace Microsoft.ComponentDetection.Orchestrator.Tests.Experiments; + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Security.Cryptography; +using Microsoft.ComponentDetection.Contracts; +using Microsoft.ComponentDetection.Contracts.TypedComponent; +using Microsoft.ComponentDetection.Orchestrator.Experiments.Models; + +public static class ExperimentTestUtils +{ + public static DetectedComponent CreateRandomComponent() => new(new NpmComponent(Guid.NewGuid().ToString(), CreateRandomVersion())); + + public static List CreateRandomComponents(int length = 5) => + Enumerable.Range(0, length).Select(_ => CreateRandomComponent()).ToList(); + + public static List CreateRandomExperimentComponents(int length = 5) => + CreateRandomComponents(length).Select(x => new ExperimentComponent(x)).ToList(); + + private static string CreateRandomVersion() => + $"{RandomNumberGenerator.GetInt32(0, 100)}.{RandomNumberGenerator.GetInt32(0, 100)}.{RandomNumberGenerator.GetInt32(0, 100)}"; +} From 483d48acc120513eebf441fcb68a9e38b713e447 Mon Sep 17 00:00:00 2001 From: Justin Perez Date: Thu, 13 Apr 2023 10:25:01 -0700 Subject: [PATCH 08/15] test: `DefaultExperimentProcessor` --- .../DefaultExperimentProcessorTests.cs | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/DefaultExperimentProcessorTests.cs diff --git a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/DefaultExperimentProcessorTests.cs b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/DefaultExperimentProcessorTests.cs new file mode 100644 index 000000000..c719d4a59 --- /dev/null +++ b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/DefaultExperimentProcessorTests.cs @@ -0,0 +1,48 @@ +namespace Microsoft.ComponentDetection.Orchestrator.Tests.Experiments; + +using System.Text.Json; +using System.Threading.Tasks; +using Microsoft.ComponentDetection.Common; +using Microsoft.ComponentDetection.Orchestrator.Experiments; +using Microsoft.ComponentDetection.Orchestrator.Experiments.Configs; +using Microsoft.ComponentDetection.Orchestrator.Experiments.Models; +using Microsoft.Extensions.Logging; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; + +[TestClass] +[TestCategory("Governance/All")] +[TestCategory("Governance/ComponentDetection")] +public class DefaultExperimentProcessorTests +{ + private readonly Mock fileWritingServiceMock; + private readonly DefaultExperimentProcessor processor; + + public DefaultExperimentProcessorTests() + { + var loggerMock = new Mock>(); + this.fileWritingServiceMock = new Mock(); + this.processor = new DefaultExperimentProcessor(this.fileWritingServiceMock.Object, loggerMock.Object); + } + + [TestMethod] + public async Task ProcessExperimentAsync_WritesSerializedExperimentDiffToFileAsync() + { + var config = new Mock(); + config.Setup(c => c.Name).Returns("TestExperiment"); + + var diff = new ExperimentDiff( + ExperimentTestUtils.CreateRandomExperimentComponents(), + ExperimentTestUtils.CreateRandomExperimentComponents()); + + var serializedDiff = JsonSerializer.Serialize(diff, new JsonSerializerOptions { WriteIndented = true }); + + await this.processor.ProcessExperimentAsync(config.Object, diff); + + this.fileWritingServiceMock.Verify( + f => f.WriteFileAsync( + It.Is(s => s.StartsWith($"Experiment_{config.Object.Name}_")), + serializedDiff), + Times.Once); + } +} From aa19a6b9b26d418898b8735b867be29c7dc75001 Mon Sep 17 00:00:00 2001 From: Justin Perez Date: Thu, 13 Apr 2023 10:25:15 -0700 Subject: [PATCH 09/15] test: `ExperimentDiff` --- .../Experiments/ExperimentDiffTests.cs | 126 ++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentDiffTests.cs diff --git a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentDiffTests.cs b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentDiffTests.cs new file mode 100644 index 000000000..3af3dcec9 --- /dev/null +++ b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentDiffTests.cs @@ -0,0 +1,126 @@ +namespace Microsoft.ComponentDetection.Orchestrator.Tests.Experiments; + +using System.Collections.Generic; +using System.Linq; +using FluentAssertions; +using Microsoft.ComponentDetection.Contracts; +using Microsoft.ComponentDetection.Contracts.TypedComponent; +using Microsoft.ComponentDetection.Orchestrator.Experiments.Models; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +[TestClass] +[TestCategory("Governance/All")] +[TestCategory("Governance/ComponentDetection")] +public class ExperimentDiffTests +{ + [TestMethod] + public void ExperimentDiff_DiffsAddedIds() + { + var testComponents = ExperimentTestUtils.CreateRandomExperimentComponents(); + var diff = new ExperimentDiff(Enumerable.Empty(), testComponents); + + diff.AddedIds.Should().BeEquivalentTo(testComponents.Select(x => x.Id)); + diff.RemovedIds.Should().BeEmpty(); + + diff.DevelopmentDependencyChanges.Should().BeEmpty(); + diff.AddedRootIds.Should().BeEmpty(); + diff.RemovedRootIds.Should().BeEmpty(); + } + + [TestMethod] + public void ExperimentDiff_DiffsRemovedIds() + { + var testComponents = ExperimentTestUtils.CreateRandomExperimentComponents(); + var diff = new ExperimentDiff(testComponents, Enumerable.Empty()); + + diff.RemovedIds.Should().BeEquivalentTo(testComponents.Select(x => x.Id)); + diff.AddedIds.Should().BeEmpty(); + + diff.DevelopmentDependencyChanges.Should().BeEmpty(); + diff.AddedRootIds.Should().BeEmpty(); + diff.RemovedRootIds.Should().BeEmpty(); + } + + [TestMethod] + public void ExperimentDiff_DiffsDevDependencies() + { + var detectedComponent = ExperimentTestUtils.CreateRandomComponent(); + var componentA = new DetectedComponent(detectedComponent.Component); + var componentB = new DetectedComponent(detectedComponent.Component); + + componentA.DevelopmentDependency = false; + componentB.DevelopmentDependency = true; + + var diff = new ExperimentDiff( + new[] { new ExperimentComponent(componentA) }, + new[] { new ExperimentComponent(componentB) }); + + diff.DevelopmentDependencyChanges.Should().HaveCount(1); + + var change = diff.DevelopmentDependencyChanges.First(); + change.Id.Should().Be(detectedComponent.Component.Id); + change.OldValue.Should().BeFalse(); + change.NewValue.Should().BeTrue(); + + diff.AddedIds.Should().BeEmpty(); + diff.RemovedIds.Should().BeEmpty(); + diff.AddedRootIds.Should().BeEmpty(); + diff.RemovedRootIds.Should().BeEmpty(); + } + + [TestMethod] + public void ExperimentDiff_DiffsAddedRootIds() + { + var rootComponent = ExperimentTestUtils.CreateRandomComponent(); + var component = ExperimentTestUtils.CreateRandomComponent(); + + var componentA = new DetectedComponent(component.Component); + var componentB = new DetectedComponent(component.Component) + { + DependencyRoots = new HashSet { rootComponent.Component }, + }; + + var diff = new ExperimentDiff( + new[] { new ExperimentComponent(componentA), }, + new[] { new ExperimentComponent(componentB), }); + + diff.AddedRootIds.Should().HaveCount(1); + diff.RemovedRootIds.Should().BeEmpty(); + + var addedRoot = diff.AddedRootIds[component.Component.Id]; + addedRoot.Should().HaveCount(1); + addedRoot.Should().BeEquivalentTo(rootComponent.Component.Id); + + diff.AddedIds.Should().BeEmpty(); + diff.RemovedIds.Should().BeEmpty(); + diff.DevelopmentDependencyChanges.Should().BeEmpty(); + } + + [TestMethod] + public void ExperimentDiff_DiffsRemovedRootIds() + { + var rootComponent = ExperimentTestUtils.CreateRandomComponent(); + var component = ExperimentTestUtils.CreateRandomComponent(); + + var componentA = new DetectedComponent(component.Component) + { + DependencyRoots = new HashSet { rootComponent.Component }, + }; + var componentB = new DetectedComponent(component.Component); + + var diff = new ExperimentDiff( + new[] { new ExperimentComponent(componentA), }, + new[] { new ExperimentComponent(componentB), }); + + diff.RemovedRootIds.Should().HaveCount(1); + diff.AddedRootIds.Should().BeEmpty(); + + var removedRoot = diff.RemovedRootIds[component.Component.Id]; + removedRoot.Should().HaveCount(1); + removedRoot.Should().BeEquivalentTo(rootComponent.Component.Id); + + diff.AddedIds.Should().BeEmpty(); + diff.RemovedIds.Should().BeEmpty(); + diff.DevelopmentDependencyChanges.Should().BeEmpty(); + } +} From a1d0e0289087f1429019450e8fc86a306d698b1b Mon Sep 17 00:00:00 2001 From: Justin Perez Date: Thu, 13 Apr 2023 10:25:33 -0700 Subject: [PATCH 10/15] test: `Experiment` --- .../Experiments/ExperimentTests.cs | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentTests.cs diff --git a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentTests.cs b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentTests.cs new file mode 100644 index 000000000..2db9798a7 --- /dev/null +++ b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentTests.cs @@ -0,0 +1,35 @@ +namespace Microsoft.ComponentDetection.Orchestrator.Tests.Experiments; + +using FluentAssertions; +using Microsoft.ComponentDetection.Orchestrator.Experiments.Models; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +[TestClass] +[TestCategory("Governance/All")] +[TestCategory("Governance/ComponentDetection")] +public class ExperimentTests +{ + [TestMethod] + public void Experiment_AddsToOnlyControlGroup() + { + var experiment = new Experiment(); + var testComponents = ExperimentTestUtils.CreateRandomComponents(); + + experiment.AddComponentsToControlGroup(testComponents); + + experiment.ControlGroupComponents.Should().HaveCount(testComponents.Count); + experiment.ExperimentGroupComponents.Should().BeEmpty(); + } + + [TestMethod] + public void Experiment_AddsToOnlyExperimentGroup() + { + var experiment = new Experiment(); + var testComponents = ExperimentTestUtils.CreateRandomComponents(); + + experiment.AddComponentsToExperimentalGroup(testComponents); + + experiment.ControlGroupComponents.Should().BeEmpty(); + experiment.ExperimentGroupComponents.Should().HaveCount(testComponents.Count); + } +} From 61fca294eec566e13c50165fbd5780828be69b1e Mon Sep 17 00:00:00 2001 From: Justin Perez Date: Thu, 13 Apr 2023 10:26:07 -0700 Subject: [PATCH 11/15] refactor: use `ExperimentTestUtils` --- .../Experiments/ExperimentServiceTests.cs | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentServiceTests.cs b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentServiceTests.cs index e6b40da43..4939ad536 100644 --- a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentServiceTests.cs +++ b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentServiceTests.cs @@ -1,10 +1,8 @@ namespace Microsoft.ComponentDetection.Orchestrator.Tests.Experiments; -using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; using Microsoft.ComponentDetection.Contracts; -using Microsoft.ComponentDetection.Contracts.TypedComponent; using Microsoft.ComponentDetection.Orchestrator.Experiments; using Microsoft.ComponentDetection.Orchestrator.Experiments.Configs; using Microsoft.ComponentDetection.Orchestrator.Experiments.Models; @@ -33,11 +31,7 @@ public ExperimentServiceTests() [TestMethod] public void RecordDetectorRun_AddsComponentsToControlAndExperimentGroup() { - var components = new List - { - new(new NpmComponent("ComponentA", "1.0.0")), - new(new NpmComponent("ComponentB", "1.0.0")), - }; + var components = ExperimentTestUtils.CreateRandomComponents(); this.experimentConfigMock.Setup(x => x.IsInControlGroup(this.detectorMock.Object)).Returns(true); this.experimentConfigMock.Setup(x => x.IsInExperimentGroup(this.detectorMock.Object)).Returns(true); @@ -56,11 +50,7 @@ public void RecordDetectorRun_AddsComponentsToControlAndExperimentGroup() [TestMethod] public async Task FinishAsync_ProcessesExperimentsAsync() { - var components = new List - { - new(new NpmComponent("ComponentA", "1.0.0")), - new(new NpmComponent("ComponentB", "1.0.0")), - }; + var components = ExperimentTestUtils.CreateRandomComponents(); this.experimentConfigMock.Setup(x => x.IsInControlGroup(this.detectorMock.Object)).Returns(true); this.experimentConfigMock.Setup(x => x.IsInExperimentGroup(this.detectorMock.Object)).Returns(true); From 010938e5b272218736419a085391b03b13987186 Mon Sep 17 00:00:00 2001 From: Justin Perez Date: Thu, 13 Apr 2023 10:26:38 -0700 Subject: [PATCH 12/15] fix: `Experiment` docs formatting --- .../Experiments/Models/Experiment.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/Experiment.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/Experiment.cs index c41067e7c..6878902ac 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/Experiment.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/Experiment.cs @@ -6,7 +6,8 @@ using Microsoft.ComponentDetection.Contracts; /// -/// Stores the results of a detector execution for an experiment. Buckets components into a control group and an experimental group. +/// Stores the results of a detector execution for an experiment. Buckets components into a control group and an +/// experimental group. /// public class Experiment { From 20974bfea9faa3fc40c2336152bd54ff1bdc23cc Mon Sep 17 00:00:00 2001 From: Justin Perez Date: Thu, 13 Apr 2023 11:31:26 -0700 Subject: [PATCH 13/15] fix: add lockfile v3 experiment --- .../Configs/NpmLockfile3Experiment.cs | 19 +++++++++++++++++++ .../Extensions/ServiceCollectionExtensions.cs | 1 + 2 files changed, 20 insertions(+) create mode 100644 src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/NpmLockfile3Experiment.cs diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/NpmLockfile3Experiment.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/NpmLockfile3Experiment.cs new file mode 100644 index 000000000..82eaea030 --- /dev/null +++ b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/NpmLockfile3Experiment.cs @@ -0,0 +1,19 @@ +namespace Microsoft.ComponentDetection.Orchestrator.Experiments.Configs; + +using Microsoft.ComponentDetection.Contracts; +using Microsoft.ComponentDetection.Detectors.Npm; + +/// +/// Validating the . +/// +public class NpmLockfile3Experiment : IExperimentConfiguration +{ + /// + public string Name => "LockfileVersion3"; + + /// + public bool IsInControlGroup(IComponentDetector componentDetector) => componentDetector is NpmComponentDetector; + + /// + public bool IsInExperimentGroup(IComponentDetector componentDetector) => componentDetector is NpmLockfile3Detector; +} diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Extensions/ServiceCollectionExtensions.cs b/src/Microsoft.ComponentDetection.Orchestrator/Extensions/ServiceCollectionExtensions.cs index e0ff6cf2f..9ee129ee3 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/Extensions/ServiceCollectionExtensions.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/Extensions/ServiceCollectionExtensions.cs @@ -71,6 +71,7 @@ public static IServiceCollection AddComponentDetection(this IServiceCollection s services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); // Detectors // CocoaPods From 4308602ec4c9c6c629984c446a32f24e29389bc9 Mon Sep 17 00:00:00 2001 From: Justin Perez Date: Thu, 13 Apr 2023 15:26:00 -0700 Subject: [PATCH 14/15] refactor: rename `Experiment` --- .../Experiments/ExperimentService.cs | 10 +++++----- .../Models/{Experiment.cs => ExperimentResults.cs} | 2 +- .../{ExperimentTests.cs => ExperimentResultsTests.cs} | 10 +++++----- 3 files changed, 11 insertions(+), 11 deletions(-) rename src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/{Experiment.cs => ExperimentResults.cs} (98%) rename test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/{ExperimentTests.cs => ExperimentResultsTests.cs} (78%) diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/ExperimentService.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/ExperimentService.cs index 7bb32fe2b..017780a7d 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/ExperimentService.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/ExperimentService.cs @@ -11,7 +11,7 @@ /// public class ExperimentService : IExperimentService { - private readonly List<(IExperimentConfiguration Config, Experiment Experiment)> experiments; + private readonly List<(IExperimentConfiguration Config, ExperimentResults ExperimentResults)> experiments; private readonly IEnumerable experimentProcessors; private readonly ILogger logger; @@ -26,7 +26,7 @@ public ExperimentService( IEnumerable experimentProcessors, ILogger logger) { - this.experiments = configs.Select(x => (x, new Experiment())).ToList(); + this.experiments = configs.Select(x => (x, new ExperimentResults())).ToList(); this.experimentProcessors = experimentProcessors; this.logger = logger; } @@ -34,11 +34,11 @@ public ExperimentService( /// public void RecordDetectorRun(IComponentDetector detector, IEnumerable components) { - foreach (var (config, experiment) in this.experiments) + foreach (var (config, experimentResults) in this.experiments) { if (config.IsInControlGroup(detector)) { - experiment.AddComponentsToControlGroup(components); + experimentResults.AddComponentsToControlGroup(components); this.logger.LogDebug( "Adding {Count} Components from {Id} to Control Group for {Experiment}", components.Count(), @@ -48,7 +48,7 @@ public void RecordDetectorRun(IComponentDetector detector, IEnumerable -public class Experiment +public class ExperimentResults { private readonly HashSet controlGroupComponents = new(); diff --git a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentTests.cs b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentResultsTests.cs similarity index 78% rename from test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentTests.cs rename to test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentResultsTests.cs index 2db9798a7..7e841ea5e 100644 --- a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentTests.cs +++ b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentResultsTests.cs @@ -7,12 +7,12 @@ [TestClass] [TestCategory("Governance/All")] [TestCategory("Governance/ComponentDetection")] -public class ExperimentTests +public class ExperimentResultsTests { [TestMethod] - public void Experiment_AddsToOnlyControlGroup() + public void ExperimentResults_AddsToOnlyControlGroup() { - var experiment = new Experiment(); + var experiment = new ExperimentResults(); var testComponents = ExperimentTestUtils.CreateRandomComponents(); experiment.AddComponentsToControlGroup(testComponents); @@ -22,9 +22,9 @@ public void Experiment_AddsToOnlyControlGroup() } [TestMethod] - public void Experiment_AddsToOnlyExperimentGroup() + public void ExperimentResults_AddsToOnlyExperimentGroup() { - var experiment = new Experiment(); + var experiment = new ExperimentResults(); var testComponents = ExperimentTestUtils.CreateRandomComponents(); experiment.AddComponentsToExperimentalGroup(testComponents); From c8eb8d88e7788b765663fc4318f3e3131561cfd7 Mon Sep 17 00:00:00 2001 From: Justin Perez Date: Thu, 13 Apr 2023 15:27:33 -0700 Subject: [PATCH 15/15] refactor: proper ctor arg names --- .../Experiments/Models/ExperimentDiff.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/ExperimentDiff.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/ExperimentDiff.cs index 267e1635d..496426d51 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/ExperimentDiff.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/ExperimentDiff.cs @@ -11,14 +11,14 @@ public class ExperimentDiff /// /// Creates a new . /// - /// A set of components from the control group. - /// A set of components from the experimental group. + /// A set of components from the control group. + /// A set of components from the experimental group. public ExperimentDiff( - IEnumerable oldComponents, - IEnumerable newComponents) + IEnumerable controlGroupComponents, + IEnumerable experimentGroupComponents) { - var oldComponentDictionary = oldComponents.ToDictionary(x => x.Id); - var newComponentDictionary = newComponents.ToDictionary(x => x.Id); + var oldComponentDictionary = controlGroupComponents.ToDictionary(x => x.Id); + var newComponentDictionary = experimentGroupComponents.ToDictionary(x => x.Id); this.AddedIds = newComponentDictionary.Keys.Except(oldComponentDictionary.Keys).ToList(); this.RemovedIds = oldComponentDictionary.Keys.Except(newComponentDictionary.Keys).ToList();