From 949ce3b520a413bdb311ba4c0889f53b24fbd680 Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Tue, 19 May 2020 11:38:46 +1000 Subject: [PATCH] Downgrade validation from exception to assertion Fixes #6204 Fixes AB#1126588 We are seeing some customer NFEs around this check. The validation is theoretically correct, but the consequences of continuing in such a scenario are minor-to-non-existant. Therefore I'm leaving it as an assert to assist during development of this code. This code would likely disappear as part of #6183 --- .../Dependencies/Snapshot/DependenciesSnapshot.cs | 12 +++++------- .../Snapshot/DependenciesSnapshotTests.cs | 9 ++++----- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/Snapshot/DependenciesSnapshot.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/Snapshot/DependenciesSnapshot.cs index d447e9695bf..75aa0f1c268 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/Snapshot/DependenciesSnapshot.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/Snapshot/DependenciesSnapshot.cs @@ -165,13 +165,11 @@ internal DependenciesSnapshot( Requires.NotNull(activeTargetFramework, nameof(activeTargetFramework)); Requires.NotNull(dependenciesByTargetFramework, nameof(dependenciesByTargetFramework)); - if (!activeTargetFramework.Equals(TargetFramework.Empty)) - { - Requires.Argument( - dependenciesByTargetFramework.ContainsKey(activeTargetFramework), - nameof(dependenciesByTargetFramework), - $"Must contain {nameof(activeTargetFramework)} ({activeTargetFramework.FullName})."); - } + // NOTE this used to throw but caused customer NFEs (changed to not throw in #6205) + System.Diagnostics.Debug.Assert( + activeTargetFramework.Equals(TargetFramework.Empty) || dependenciesByTargetFramework.ContainsKey(activeTargetFramework), + $"Active target framework {activeTargetFramework.FullName} ({activeTargetFramework.ShortName}) " + + "is not present in target framework map: " + string.Join(", ", dependenciesByTargetFramework.Keys.Select(tf => tf.FullName))); ActiveTargetFramework = activeTargetFramework; DependenciesByTargetFramework = dependenciesByTargetFramework; diff --git a/tests/Microsoft.VisualStudio.ProjectSystem.Managed.UnitTests/ProjectSystem/Tree/Dependencies/Snapshot/DependenciesSnapshotTests.cs b/tests/Microsoft.VisualStudio.ProjectSystem.Managed.UnitTests/ProjectSystem/Tree/Dependencies/Snapshot/DependenciesSnapshotTests.cs index c3aee297284..840deb9e79c 100644 --- a/tests/Microsoft.VisualStudio.ProjectSystem.Managed.UnitTests/ProjectSystem/Tree/Dependencies/Snapshot/DependenciesSnapshotTests.cs +++ b/tests/Microsoft.VisualStudio.ProjectSystem.Managed.UnitTests/ProjectSystem/Tree/Dependencies/Snapshot/DependenciesSnapshotTests.cs @@ -24,15 +24,14 @@ public void Constructor_WhenRequiredParamsNotProvided_ShouldThrow() } [Fact] - public void Constructor_ThrowsIfActiveTargetFrameworkNotEmptyAndNotInDependenciesByTargetFramework() + public void Constructor_ActiveTargetFrameworkNotEmptyAndNotInDependenciesByTargetFramework() { + // NOTE this used to throw but caused customer NFEs (changed to not throw in #6205) var targetFramework = new TargetFramework("tfm1"); - var ex = Assert.Throws(() => new DependenciesSnapshot( + _ = new DependenciesSnapshot( activeTargetFramework: targetFramework, - ImmutableDictionary.Empty)); - - Assert.StartsWith("Must contain activeTargetFramework (tfm1).", ex.Message); + ImmutableDictionary.Empty); } [Fact]