Downgrade validation from exception to assertion

Fixes dotnet#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 dotnet#6183
drewnoakes · May 19, 2020 · 949ce3b · 949ce3b
1 parent bad560e
commit 949ce3b
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 12 deletions.
diff --git a/...io.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/Snapshot/DependenciesSnapshot.cs b/...io.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/...m.Managed.UnitTests/ProjectSystem/Tree/Dependencies/Snapshot/DependenciesSnapshotTests.cs b/...m.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<ArgumentException>(() => new DependenciesSnapshot(
+            _ = new DependenciesSnapshot(
                 activeTargetFramework: targetFramework,
-                ImmutableDictionary<ITargetFramework, TargetedDependenciesSnapshot>.Empty));
-
-            Assert.StartsWith("Must contain activeTargetFramework (tfm1).", ex.Message);
+                ImmutableDictionary<ITargetFramework, TargetedDependenciesSnapshot>.Empty);
         }
 
         [Fact]