Copy acceptance test diagnostic logs into the test results directory

test/Microsoft.TestPlatform.Acceptance.IntegrationTests/AttachmentUtilsTests.cs

@@ -0,0 +1,127 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Text;
+
+using Microsoft.TestPlatform.TestUtilities;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+namespace Microsoft.TestPlatform.AcceptanceTests;
+
+[TestClass]
+public class AttachmentUtilsTests
+{
+    private readonly List<string> _log = new();
+    private TempDirectory _temp = null!;
+
+    [TestInitialize]
+    public void Initialize()
+    {
+        TempDirectory.NuGetConfigPath = Path.Combine(IntegrationTestEnvironment.RepoRootDirectory, "NuGet.config");
+        _temp = new TempDirectory();
+    }
+
+    [TestCleanup]
+    public void Cleanup() => _temp.Dispose();
+
+    [TestMethod]
+    [DataRow("Simple", "Simple")]
+    [DataRow("with space", "with_space")]
+    [DataRow("a:b/c\\d", "a_b_c_d")]
+    [DataRow("...", "test")]
+    [DataRow("", "test")]
+    [DataRow(null, "test")]
+    public void SanitizePathSegmentReplacesEveryRunOfUnsafeCharactersWithOneUnderscore(string? name, string expected)
+        => Assert.AreEqual(expected, AttachmentUtils.SanitizePathSegment(name));
+
+    [TestMethod]
+    public void SanitizePathSegmentShortensTheNameOfADataDrivenTest()
+    {
+        var sanitized = AttachmentUtils.SanitizePathSegment("CrashDumpOnStackOverflow (Row: 0, Runner = net10.0, TargetFramework = net10.0, InProcess)");
+
+        Assert.IsTrue(sanitized.StartsWith("CrashDumpOnStackOverflow_Row_0_Runner_net10.0", StringComparison.Ordinal), sanitized);
+        Assert.IsLessThanOrEqualTo(AttachmentUtils.MaxNameLength, sanitized.Length, sanitized);
+        Assert.AreEqual(-1, sanitized.IndexOfAny(Path.GetInvalidFileNameChars()), sanitized);
+    }
+
+    [TestMethod]
+    public void CopyAttachmentsCopiesTheWholeTreeOfAnAttachedDirectory()
+    {
+        var logs = _temp.CreateDirectory("logs").FullName;
+        var destination = _temp.CreateDirectory("destination").FullName;
+        File.WriteAllText(Path.Combine(logs, "log.txt"), "runner");
+        File.WriteAllText(Path.Combine(Directory.CreateDirectory(Path.Combine(logs, "nested")).FullName, "log.host.txt"), "host");
+
+        var copies = AttachmentUtils.CopyAttachments(new[] { logs }, destination, _log.Add);
+
+        CollectionAssert.AreEquivalent(
+            new[] { Path.Combine(destination, "log.txt"), Path.Combine(destination, "nested", "log.host.txt") },
+            copies,
+            Logged());
+        Assert.AreEqual("runner", File.ReadAllText(Path.Combine(destination, "log.txt")));
+        Assert.AreEqual("host", File.ReadAllText(Path.Combine(destination, "nested", "log.host.txt")));
+    }
+
+    [TestMethod]
+    public void CopyAttachmentsCopiesALogThatIsStillOpenForWriting()
+    {
+        var logs = _temp.CreateDirectory("logs").FullName;
+        var destination = _temp.CreateDirectory("destination").FullName;
+
+        // A testhost that did not exit yet keeps its diagnostic log open, and shares it only for reading.
+        // File.Copy asks for more than that and fails on exactly the logs we care about the most.
+        using var open = new FileStream(Path.Combine(logs, "log.txt"), FileMode.CreateNew, FileAccess.Write, FileShare.Read);
+        var content = Encoding.UTF8.GetBytes("still writing");
+        open.Write(content, 0, content.Length);
+        open.Flush();
+
+        var copies = AttachmentUtils.CopyAttachments(new[] { logs }, destination, _log.Add);
+
+        Assert.HasCount(1, copies, Logged());
+        Assert.AreEqual("still writing", File.ReadAllText(copies[0]));
+    }
+
+    [TestMethod]
+    public void CopyAttachmentsSkipsAnAttachmentThatIsNotThere()
+    {
+        var destination = _temp.CreateDirectory("destination").FullName;
+
+        var copies = AttachmentUtils.CopyAttachments(new[] { Path.Combine(_temp.Path, "gone") }, destination, _log.Add);
+
+        Assert.IsEmpty(copies, Logged());
+        Assert.IsEmpty(Directory.GetFileSystemEntries(destination));
+    }
+
+    [TestMethod]
+    public void CopyAttachmentsCopiesTheSameDirectoryOnlyOnce()
+    {
+        var logs = _temp.CreateDirectory("logs").FullName;
+        var destination = _temp.CreateDirectory("destination").FullName;
+        File.WriteAllText(Path.Combine(logs, "log.txt"), "runner");
+
+        var copies = AttachmentUtils.CopyAttachments(new[] { logs, logs }, destination, _log.Add);
+
+        Assert.HasCount(1, copies, Logged());
+    }
+
+    [TestMethod]
+    public void CopyAttachmentsKeepsBothFilesWhenTwoAttachmentsAreNamedTheSame()
+    {
+        var first = _temp.CreateDirectory("first").FullName;
+        var second = _temp.CreateDirectory("second").FullName;
+        var destination = _temp.CreateDirectory("destination").FullName;
+        File.WriteAllText(Path.Combine(first, "log.txt"), "first");
+        File.WriteAllText(Path.Combine(second, "log.txt"), "second");
+
+        var copies = AttachmentUtils.CopyAttachments(new[] { first, second }, destination, _log.Add);
+
+        Assert.HasCount(2, copies, Logged());
+        CollectionAssert.AreEquivalent(new[] { "first", "second" }, copies.Select(File.ReadAllText).ToList());
+    }
+
+    private string Logged() => string.Join(Environment.NewLine, _log);
+}

test/Microsoft.TestPlatform.TestUtilities/AttachmentUtils.cs

@@ -0,0 +1,176 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Text;
+
+namespace Microsoft.TestPlatform.TestUtilities;
+
+/// <summary>
+/// Copies the files a failed test wants to attach into the directory where the test results are written.
+/// <c>TestContext.AddResultFile</c> only records the path it is given, and the diagnostic logs are written
+/// into a temporary directory that CI does not publish, so attaching them where they are leaves the trx
+/// pointing at files that no longer exist by the time anyone reads it.
+/// </summary>
+public static class AttachmentUtils
+{
+    /// <summary>
+    /// How much of a test name we keep when we use it as a directory name. The names of data driven tests
+    /// contain the whole data row, and the resulting path has to stay within the limits of the file system.
+    /// </summary>
+    public const int MaxNameLength = 60;
+
+    /// <summary>
+    /// Replaces the characters that are not safe in a path, and shortens the name, so it can be used as a
+    /// single directory or file name.
+    /// </summary>
+    public static string SanitizePathSegment(string? name, int maxLength = MaxNameLength)
+    {
+        var builder = new StringBuilder(maxLength);
+        foreach (var character in name ?? string.Empty)
+        {
+            var safe = char.IsLetterOrDigit(character) || character is '.' or '-' or '_' ? character : '_';
+
+            // One separator is enough, the names of data driven tests are full of spaces and punctuation.
+            if (safe == '_' && builder.Length > 0 && builder[builder.Length - 1] == '_')
+            {
+                continue;
+            }
+
+            builder.Append(safe);
+            if (builder.Length == maxLength)
+            {
+                break;
+            }
+        }
+
+        // Windows does not allow a name to end with a dot or a space.
+        var sanitized = builder.ToString().Trim('_', '.', ' ');
+        return sanitized.Length == 0 ? "test" : sanitized;
+    }
+
+    /// <summary>
+    /// Returns every file in the given attachments, which are files or directories, together with the path
+    /// the file should get relative to the directory we copy it into. Directories that cannot be listed are
+    /// reported to <paramref name="log"/> and skipped.
+    /// </summary>
+    public static List<(string Path, string RelativePath)> EnumerateAttachments(IEnumerable<string> attachments, Action<string> log)
+    {
+        var files = new List<(string Path, string RelativePath)>();
+
+        // The same log directory is added once per run of vstest.console, and a test can run it more than once.
+        var seen = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
+
+        foreach (var attachment in attachments)
+        {
+            if (Directory.Exists(attachment))
+            {
+                var root = attachment.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
+                foreach (var file in EnumerateFiles(root, log))
+                {
+                    if (seen.Add(file))
+                    {
+                        files.Add((file, GetRelativePath(root, file)));
+                    }
+                }
+            }
+            else if (File.Exists(attachment) && seen.Add(attachment))
+            {
+                files.Add((attachment, Path.GetFileName(attachment)));
+            }
+        }
+
+        return files;
+    }
+
+    /// <summary>
+    /// Copies the given attachments into <paramref name="destinationDirectory"/> and returns the paths of the
+    /// copies. A file that cannot be copied is reported to <paramref name="log"/> and skipped. This runs in test
+    /// cleanup, where throwing would replace the failure the test reported with an unrelated one.
+    /// </summary>
+    public static List<string> CopyAttachments(IEnumerable<string> attachments, string destinationDirectory, Action<string> log)
+    {
+        var copies = new List<string>();
+        foreach (var (path, relativePath) in EnumerateAttachments(attachments, log))
+        {
+            var copy = CopyFile(path, Path.Combine(destinationDirectory, relativePath), log);
+            if (copy is not null)
+            {
+                copies.Add(copy);
+            }
+        }
+
+        return copies;
+    }
+
+    private static string? CopyFile(string source, string destination, Action<string> log)
+    {
+        try
+        {
+            destination = GetNonExistingPath(destination);
+            var directory = Path.GetDirectoryName(destination);
+            if (directory is not null)
+            {
+                Directory.CreateDirectory(directory);
+            }
+
+            // A process that did not exit yet can still hold the log file open, and File.Copy asks for more
+            // access than it needs, so it fails on exactly the logs we care about the most.
+            using (var from = new FileStream(source, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete))
+            using (var to = new FileStream(destination, FileMode.CreateNew, FileAccess.Write, FileShare.None))
+            {
+                from.CopyTo(to);
+            }
+
+            return destination;
+        }
+        catch (Exception ex)
+        {
+            log($"Could not copy '{source}' to '{destination}': {ex.Message}");
+            return null;
+        }
+    }
+
+    private static IEnumerable<string> EnumerateFiles(string directory, Action<string> log)
+    {
+        try
+        {
+            // Materialize the result, an error in the middle of a lazy enumeration would escape this try.
+            return new List<string>(Directory.EnumerateFiles(directory, "*", SearchOption.AllDirectories));
+        }
+        catch (Exception ex)
+        {
+            log($"Could not list the files in '{directory}': {ex.Message}");
+            return new List<string>();
+        }
+    }
+
+    private static string GetRelativePath(string root, string file)
+        => file.Length > root.Length && file.StartsWith(root, StringComparison.OrdinalIgnoreCase)
+            ? file.Substring(root.Length).TrimStart(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)
+            : Path.GetFileName(file);
+
+    private static string GetNonExistingPath(string path)
+    {
+        if (!File.Exists(path))
+        {
+            return path;
+        }
+
+        var directory = Path.GetDirectoryName(path) ?? string.Empty;
+        var name = Path.GetFileNameWithoutExtension(path);
+        var extension = Path.GetExtension(path);
+        for (var suffix = 2; suffix <= 100; suffix++)
+        {
+            var candidate = Path.Combine(directory, $"{name}_{suffix}{extension}");
+            if (!File.Exists(candidate))
+            {
+                return candidate;
+            }
+        }
+
+        return Path.Combine(directory, $"{name}_{RandomId.Next()}{extension}");
+    }
+}

test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBase.cs

@@ -109,21 +109,62 @@ public void IntegrationTestBaseTestCleanup()
             return;
         }
 
-        // Attach files that are of interest.
-        foreach (var attachment in _attachments)
+        AttachFilesOfInterest();
+    }
+
+    /// <summary>
+    /// Attaches the diagnostic logs of a failed test to the test result. <c>AddResultFile</c> only records the path it
+    /// is given, and the logs are written under the temp directory, which CI does not publish and which is deleted when
+    /// the build agent is recycled. Copy them into the test results directory first and attach the copies, so the paths
+    /// in the trx still point at files that exist.
+    /// </summary>
+    private void AttachFilesOfInterest()
+    {
+        var destination = TryCreateAttachmentDirectory();
+        if (destination is null)
         {
-            if (Directory.Exists(attachment))
+            // We don't know where to copy them, attach the originals, they are still on disk for a local run.
+            foreach (var attachment in AttachmentUtils.EnumerateAttachments(_attachments, Console.WriteLine))
             {
-                foreach (var file in Directory.EnumerateFiles(attachment, "*.*", SearchOption.AllDirectories))
-                {
-                    TestContext.AddResultFile(file);
-                }
+                TestContext.AddResultFile(attachment.Path);
             }
 
-            if (File.Exists(attachment))
+            return;
+        }
+
+        foreach (var copy in AttachmentUtils.CopyAttachments(_attachments, destination, Console.WriteLine))
+        {
+            TestContext.AddResultFile(copy);
+        }
+    }
+
+    /// <summary>
+    /// Creates a directory for the files attached by the current test, under the directory where the test results are
+    /// written. Returns null when that directory is not known, or cannot be created.
+    /// </summary>
+    private string? TryCreateAttachmentDirectory()
+    {
+        try
+        {
+            var resultsDirectory = TestContext.TestRunResultsDirectory ?? TestContext.TestResultsDirectory ?? TestContext.TestRunDirectory;
+            if (StringUtils.IsNullOrWhiteSpace(resultsDirectory))
             {
-                TestContext.AddResultFile(attachment);
+                Console.WriteLine("Not copying the attached files, TestContext does not say where the test results are written.");
+                return null;
             }
+
+            // Acceptance tests run in parallel and every one of them writes a file called log.txt, so each test needs
+            // a directory of its own.
+            var name = $"{AttachmentUtils.SanitizePathSegment(TestContext.TestName)}_{RandomId.Next()}";
+            var directory = Path.Combine(resultsDirectory, "attachments", name);
+            Directory.CreateDirectory(directory);
+            Console.WriteLine($"Copying the attached files to: {directory}");
+            return directory;
+        }
+        catch (Exception ex)
+        {
+            Console.WriteLine($"Could not create the directory for the attached files: {ex}");
+            return null;
         }
     }