From 40492973be6075efbfbd231aed7168194d36cc73 Mon Sep 17 00:00:00 2001
From: Rob Mensching <rob@firegiant.com>
Date: Thu, 10 Feb 2022 19:19:46 -0800
Subject: Put validation tests behind #if due to confirmed MSI inconsistencies

When the validation tests are run repeatedly, they will expose an issue
in the Windows Installer where ICE validations will occasionally
fail to send all error/warning messages. The inconsistency obviously
wreaks havoc on the tests.

The tests are still interesting (and complex) enough to keep around
for manual testing. So they were placed behind an #if for easy manual
inclusion.

Note that these are "negative tests" that expose the Windows Installer
inconsistency when expected failures are missing. Other validation tests
are "positive tests" that look for success and thus will always
succeed. Therefore, the positive tests stay active.
---
 .../ValidationFixture.cs                           | 10 +++
 src/wix/test/WixToolsetTest.Sdk/MsbuildFixture.cs  | 78 -----------------
 .../WixToolsetTest.Sdk/MsbuildValidationFixture.cs | 99 ++++++++++++++++++++++
 3 files changed, 109 insertions(+), 78 deletions(-)
 create mode 100644 src/wix/test/WixToolsetTest.Sdk/MsbuildValidationFixture.cs

diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/ValidationFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/ValidationFixture.cs
index fd8f6f87..72397547 100644
--- a/src/wix/test/WixToolsetTest.CoreIntegration/ValidationFixture.cs
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/ValidationFixture.cs
@@ -12,6 +12,15 @@ namespace WixToolsetTest.CoreIntegration
     using WixToolset.Data.WindowsInstaller;
     using Xunit;
 
+    // When these tests are run repeatedly, they will expose an issue
+    // in the Windows Installer where ICE validations will occasionally
+    // fail to send all error/warning messages. The inconsistency
+    // obviously wreaks havoc on the tests.
+    //
+    // These tests are still interesting (and complex) enough to keep
+    // around for manual testing. Uncomment or define the following
+    // line to do so.
+#if DISABLE_VALIDATION_TESTS_DUE_TO_WINDOWS_INSTALLER_INCONSISTENCIES
     public class ValidationFixture
     {
         [Fact]
@@ -303,4 +312,5 @@ namespace WixToolsetTest.CoreIntegration
             }
         }
     }
+#endif
 }
diff --git a/src/wix/test/WixToolsetTest.Sdk/MsbuildFixture.cs b/src/wix/test/WixToolsetTest.Sdk/MsbuildFixture.cs
index f0761762..0f7f8a88 100644
--- a/src/wix/test/WixToolsetTest.Sdk/MsbuildFixture.cs
+++ b/src/wix/test/WixToolsetTest.Sdk/MsbuildFixture.cs
@@ -6,8 +6,6 @@ namespace WixToolsetTest.Sdk
     using System.Collections.Generic;
     using System.IO;
     using System.Linq;
-    using System.Runtime.CompilerServices;
-    using System.Threading;
     using WixBuildTools.TestSupport;
     using Xunit;
 
@@ -322,82 +320,6 @@ namespace WixToolsetTest.Sdk
             }
         }
 
-        [Theory]
-        [InlineData(BuildSystem.DotNetCoreSdk)]
-        [InlineData(BuildSystem.MSBuild)]
-        [InlineData(BuildSystem.MSBuild64)]
-        public void CannotBuildMsiPackageWithIceIssues(BuildSystem buildSystem)
-        {
-            var sourceFolder = TestData.Get(@"TestData\MsiPackageWithIceError\MsiPackage");
-
-            var testLogsFolder = TestData.GetUnitTestLogsFolder();
-            File.Delete(Path.Combine(testLogsFolder, buildSystem + ".binlog"));
-            File.Delete(Path.Combine(testLogsFolder, buildSystem + ".msi"));
-
-            using (var fs = new TestDataFolderFileSystem())
-            {
-                fs.Initialize(sourceFolder);
-                var baseFolder = fs.BaseFolder;
-                var projectPath = Path.Combine(baseFolder, "MsiPackage.wixproj");
-
-                var result = MsbuildUtilities.BuildProject(buildSystem, projectPath, suppressValidation: false);
-
-                File.Copy(Path.ChangeExtension(projectPath, ".binlog"), Path.Combine(testLogsFolder, buildSystem + ".binlog"));
-                File.Copy(Path.Combine(baseFolder, "obj", "x86", "Release", "en-US", "MsiPackage.msi"), Path.Combine(testLogsFolder, buildSystem + ".msi"));
-
-                var iceIssues = result.Output.Where(line => line.Contains(": error") || line.Contains(": warning"))
-                                             .Select(line => line.Replace(baseFolder, "<baseFolder>")
-                                                                 .Replace("1>", String.Empty)  // remove any multi-proc markers
-                                                                 .Replace("WIX204", "WIX0204") // normalize error number reporting because MSBuild is not consistent on zero padding
-                                                                 .Trim())
-                                             .OrderBy(s => s, StringComparer.OrdinalIgnoreCase)
-                                             .ToArray();
-                WixAssert.CompareLineByLine(new[]
-                {
-                    @"<baseFolder>\Package.wxs(17): error WIX0204: ICE12: CustomAction: CausesICE12Error is of type: 35. Therefore it must come after CostFinalize @ 1000 in Seq Table: InstallExecuteSequence. CA Seq#: 49 [<baseFolder>\MsiPackage.wixproj]",
-                    @"<baseFolder>\Package.wxs(17): error WIX0204: ICE12: CustomAction: CausesICE12Error is of type: 35. Therefore it must come after CostFinalize @ 1000 in Seq Table: InstallExecuteSequence. CA Seq#: 49 [<baseFolder>\MsiPackage.wixproj]",
-                    @"<baseFolder>\Package.wxs(8): warning WIX1076: ICE46: Property 'Myproperty' referenced in column 'LaunchCondition'.'Condition' of row 'Myproperty' differs from a defined property by case only. [<baseFolder>\MsiPackage.wixproj]",
-                    @"<baseFolder>\Package.wxs(8): warning WIX1076: ICE46: Property 'Myproperty' referenced in column 'LaunchCondition'.'Condition' of row 'Myproperty' differs from a defined property by case only. [<baseFolder>\MsiPackage.wixproj]",
-                }, iceIssues);
-            }
-        }
-
-        [Theory(Skip = "Flaky")]
-        [InlineData(BuildSystem.DotNetCoreSdk)]
-        [InlineData(BuildSystem.MSBuild)]
-        [InlineData(BuildSystem.MSBuild64)]
-        public void CannotBuildMsiPackageWithIceWarningsAsErrors(BuildSystem buildSystem)
-        {
-            var sourceFolder = TestData.Get(@"TestData\MsiPackageWithIceError\MsiPackage");
-
-            using (var fs = new TestDataFolderFileSystem())
-            {
-                fs.Initialize(sourceFolder);
-                var baseFolder = fs.BaseFolder;
-                var projectPath = Path.Combine(baseFolder, "MsiPackage.wixproj");
-
-                var result = MsbuildUtilities.BuildProject(buildSystem, projectPath, new[]
-                {
-                    $"-p:TreatWarningsAsErrors=true",
-                    MsbuildUtilities.GetQuotedPropertySwitch(buildSystem, "SuppressIces", "ICE12"),
-                }, suppressValidation: false);
-                Assert.Equal(1, result.ExitCode);
-
-                var iceIssues = result.Output.Where(line => (line.Contains(": error") || line.Contains(": warning")))
-                                             .Select(line => line.Replace(baseFolder, "<baseFolder>")
-                                                                 .Replace("1>", String.Empty)  // remove any multi-proc markers
-                                                                 .Replace("WIX204", "WIX0204") // normalize error number reporting because MSBuild is not consistent on zero padding
-                                                                 .Trim())
-                                             .OrderBy(s => s, StringComparer.OrdinalIgnoreCase)
-                                             .ToArray();
-                WixAssert.CompareLineByLine(new[]
-                {
-                    @"<baseFolder>\Package.wxs(8): error WIX1076: ICE46: Property 'Myproperty' referenced in column 'LaunchCondition'.'Condition' of row 'Myproperty' differs from a defined property by case only. [<baseFolder>\MsiPackage.wixproj]",
-                    @"<baseFolder>\Package.wxs(8): error WIX1076: ICE46: Property 'Myproperty' referenced in column 'LaunchCondition'.'Condition' of row 'Myproperty' differs from a defined property by case only. [<baseFolder>\MsiPackage.wixproj]",
-                }, iceIssues);
-            }
-        }
-
         [Theory]
         [InlineData(BuildSystem.DotNetCoreSdk)]
         [InlineData(BuildSystem.MSBuild)]
diff --git a/src/wix/test/WixToolsetTest.Sdk/MsbuildValidationFixture.cs b/src/wix/test/WixToolsetTest.Sdk/MsbuildValidationFixture.cs
new file mode 100644
index 00000000..829ee9c1
--- /dev/null
+++ b/src/wix/test/WixToolsetTest.Sdk/MsbuildValidationFixture.cs
@@ -0,0 +1,99 @@
+// Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the Microsoft Reciprocal License. See LICENSE.TXT file in the project root for full license information.
+
+namespace WixToolsetTest.Sdk
+{
+    using System;
+    using System.IO;
+    using System.Linq;
+    using WixBuildTools.TestSupport;
+    using Xunit;
+
+    // When these tests are run repeatedly, they will expose an issue
+    // in the Windows Installer where ICE validations will occasionally
+    // fail to send all error/warning messages. The inconsistency
+    // obviously wreaks havoc on the tests.
+    //
+    // These tests are still interesting (and complex) enough to keep
+    // around for manual testing. Uncomment or define the following
+    // line to do so.
+#if DISABLE_VALIDATION_TESTS_DUE_TO_WINDOWS_INSTALLER_INCONSISTENCIES
+    public class MsbuildValidationFixture
+    {
+        [Theory]
+        [InlineData(BuildSystem.DotNetCoreSdk)]
+        [InlineData(BuildSystem.MSBuild)]
+        [InlineData(BuildSystem.MSBuild64)]
+        public void CannotBuildMsiPackageWithIceIssues(BuildSystem buildSystem)
+        {
+            var sourceFolder = TestData.Get(@"TestData\MsiPackageWithIceError\MsiPackage");
+
+            var testLogsFolder = TestData.GetUnitTestLogsFolder();
+            File.Delete(Path.Combine(testLogsFolder, buildSystem + ".binlog"));
+            File.Delete(Path.Combine(testLogsFolder, buildSystem + ".msi"));
+
+            using (var fs = new TestDataFolderFileSystem())
+            {
+                fs.Initialize(sourceFolder);
+                var baseFolder = fs.BaseFolder;
+                var projectPath = Path.Combine(baseFolder, "MsiPackage.wixproj");
+
+                var result = MsbuildUtilities.BuildProject(buildSystem, projectPath, suppressValidation: false);
+
+                File.Copy(Path.ChangeExtension(projectPath, ".binlog"), Path.Combine(testLogsFolder, buildSystem + ".binlog"));
+                File.Copy(Path.Combine(baseFolder, "obj", "x86", "Release", "en-US", "MsiPackage.msi"), Path.Combine(testLogsFolder, buildSystem + ".msi"));
+
+                var iceIssues = result.Output.Where(line => line.Contains(": error") || line.Contains(": warning"))
+                                             .Select(line => line.Replace(baseFolder, "<baseFolder>")
+                                                                 .Replace("1>", String.Empty)  // remove any multi-proc markers
+                                                                 .Replace("WIX204", "WIX0204") // normalize error number reporting because MSBuild is not consistent on zero padding
+                                                                 .Trim())
+                                             .OrderBy(s => s, StringComparer.OrdinalIgnoreCase)
+                                             .ToArray();
+                WixAssert.CompareLineByLine(new[]
+                {
+                    @"<baseFolder>\Package.wxs(17): error WIX0204: ICE12: CustomAction: CausesICE12Error is of type: 35. Therefore it must come after CostFinalize @ 1000 in Seq Table: InstallExecuteSequence. CA Seq#: 49 [<baseFolder>\MsiPackage.wixproj]",
+                    @"<baseFolder>\Package.wxs(17): error WIX0204: ICE12: CustomAction: CausesICE12Error is of type: 35. Therefore it must come after CostFinalize @ 1000 in Seq Table: InstallExecuteSequence. CA Seq#: 49 [<baseFolder>\MsiPackage.wixproj]",
+                    @"<baseFolder>\Package.wxs(8): warning WIX1076: ICE46: Property 'Myproperty' referenced in column 'LaunchCondition'.'Condition' of row 'Myproperty' differs from a defined property by case only. [<baseFolder>\MsiPackage.wixproj]",
+                    @"<baseFolder>\Package.wxs(8): warning WIX1076: ICE46: Property 'Myproperty' referenced in column 'LaunchCondition'.'Condition' of row 'Myproperty' differs from a defined property by case only. [<baseFolder>\MsiPackage.wixproj]",
+                }, iceIssues);
+            }
+        }
+
+        [Theory]
+        [InlineData(BuildSystem.DotNetCoreSdk)]
+        [InlineData(BuildSystem.MSBuild)]
+        [InlineData(BuildSystem.MSBuild64)]
+        public void CannotBuildMsiPackageWithIceWarningsAsErrors(BuildSystem buildSystem)
+        {
+            var sourceFolder = TestData.Get(@"TestData\MsiPackageWithIceError\MsiPackage");
+
+            using (var fs = new TestDataFolderFileSystem())
+            {
+                fs.Initialize(sourceFolder);
+                var baseFolder = fs.BaseFolder;
+                var projectPath = Path.Combine(baseFolder, "MsiPackage.wixproj");
+
+                var result = MsbuildUtilities.BuildProject(buildSystem, projectPath, new[]
+                {
+                    $"-p:TreatWarningsAsErrors=true",
+                    MsbuildUtilities.GetQuotedPropertySwitch(buildSystem, "SuppressIces", "ICE12"),
+                }, suppressValidation: false);
+                Assert.Equal(1, result.ExitCode);
+
+                var iceIssues = result.Output.Where(line => (line.Contains(": error") || line.Contains(": warning")))
+                                             .Select(line => line.Replace(baseFolder, "<baseFolder>")
+                                                                 .Replace("1>", String.Empty)  // remove any multi-proc markers
+                                                                 .Replace("WIX204", "WIX0204") // normalize error number reporting because MSBuild is not consistent on zero padding
+                                                                 .Trim())
+                                             .OrderBy(s => s, StringComparer.OrdinalIgnoreCase)
+                                             .ToArray();
+                WixAssert.CompareLineByLine(new[]
+                {
+                    @"<baseFolder>\Package.wxs(8): error WIX1076: ICE46: Property 'Myproperty' referenced in column 'LaunchCondition'.'Condition' of row 'Myproperty' differs from a defined property by case only. [<baseFolder>\MsiPackage.wixproj]",
+                    @"<baseFolder>\Package.wxs(8): error WIX1076: ICE46: Property 'Myproperty' referenced in column 'LaunchCondition'.'Condition' of row 'Myproperty' differs from a defined property by case only. [<baseFolder>\MsiPackage.wixproj]",
+                }, iceIssues);
+            }
+        }
+    }
+#endif
+}
-- 
cgit v1.2.3-55-g6feb