Skip instrumentation of module with embedded ppbd without local sourc…

…es (#510) * handle ppbd without local sources * add test * nit formatting * check all docs existence * update tests
coverlet-coverage · Aug 14, 2019 · 0faaeea · 0faaeea
1 parent 872e087
commit 0faaeea
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 3 deletions.
diff --git a/src/coverlet.core/Helpers/InstrumentationHelper.cs b/src/coverlet.core/Helpers/InstrumentationHelper.cs
@@ -5,6 +5,7 @@
 using System.IO;
 using System.Linq;
 using System.Reflection;
+using System.Reflection.Metadata;
 using System.Reflection.PortableExecutable;
 using System.Text.RegularExpressions;
 using Coverlet.Core.Abstracts;
@@ -67,8 +68,9 @@ public static string[] GetCoverableModules(string module, string[] directories,
                 .ToArray();
         }
 
-        public static bool HasPdb(string module)
+        public static bool HasPdb(string module, out bool embedded)
         {
+            embedded = false;
             using (var moduleStream = File.OpenRead(module))
             using (var peReader = new PEReader(moduleStream))
             {
@@ -80,6 +82,7 @@ public static bool HasPdb(string module)
                         if (codeViewData.Path == $"{Path.GetFileNameWithoutExtension(module)}.pdb")
                         {
                             // PDB is embedded
+                            embedded = true;
                             return true;
                         }
 
@@ -91,6 +94,41 @@ public static bool HasPdb(string module)
             }
         }
 
+        public static bool EmbeddedPortablePdbHasLocalSource(string module)
+        {
+            using (FileStream moduleStream = File.OpenRead(module))
+            using (var peReader = new PEReader(moduleStream))
+            {
+                foreach (DebugDirectoryEntry entry in peReader.ReadDebugDirectory())
+                {
+                    if (entry.Type == DebugDirectoryEntryType.EmbeddedPortablePdb)
+                    {
+                        using (MetadataReaderProvider embeddedMetadataProvider = peReader.ReadEmbeddedPortablePdbDebugDirectoryData(entry))
+                        {
+                            MetadataReader metadataReader = embeddedMetadataProvider.GetMetadataReader();
+                            foreach (DocumentHandle docHandle in metadataReader.Documents)
+                            {
+                                Document document = metadataReader.GetDocument(docHandle);
+                                string docName = metadataReader.GetString(document.Name);
+
+                                // We verify all docs and return false if not all are present in local
+                                // We could have false negative if doc is not a source
+                                // Btw check for all possible extension could be weak approach
+                                if (!File.Exists(docName))
+                                {
+                                    return false;
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+
+            // If we don't have EmbeddedPortablePdb entry return true, for instance empty dll
+            // We should call this method only on embedded pdb module
+            return true;
+        }
+
         public static void BackupOriginalModule(string module, string identifier)
         {
             var backupPath = GetBackupPath(module, identifier);

diff --git a/src/coverlet.core/Instrumentation/Instrumenter.cs b/src/coverlet.core/Instrumentation/Instrumenter.cs
@@ -54,7 +54,29 @@ public bool CanInstrument()
         {
             try
             {
-                return InstrumentationHelper.HasPdb(_module);
+                if (InstrumentationHelper.HasPdb(_module, out bool embeddedPdb))
+                {
+                    if (embeddedPdb)
+                    {
+                        if (InstrumentationHelper.EmbeddedPortablePdbHasLocalSource(_module))
+                        {
+                            return true;
+                        }
+                        else
+                        {
+                            _logger.LogWarning($"Unable to instrument module: {_module}, embedded pdb without local source files");
+                            return false;
+                        }
+                    }
+                    else
+                    {
+                        return true;
+                    }
+                }
+                else
+                {
+                    return false;
+                }
             }
             catch (Exception ex)
             {

diff --git a/test/coverlet.core.tests/Helpers/InstrumentationHelperTests.cs b/test/coverlet.core.tests/Helpers/InstrumentationHelperTests.cs
@@ -27,7 +27,8 @@ public void TestGetDependenciesWithTestAssembly()
         [Fact]
         public void TestHasPdb()
         {
-            Assert.True(InstrumentationHelper.HasPdb(typeof(InstrumentationHelperTests).Assembly.Location));
+            Assert.True(InstrumentationHelper.HasPdb(typeof(InstrumentationHelperTests).Assembly.Location, out bool embeddedPdb));
+            Assert.False(embeddedPdb);
         }
 
         [Fact]

diff --git a/test/coverlet.core.tests/Instrumentation/InstrumenterTests.cs b/test/coverlet.core.tests/Instrumentation/InstrumenterTests.cs
@@ -3,6 +3,7 @@
 using System.Linq;
 using System.Reflection;
 
+using Coverlet.Core.Helpers;
 using Coverlet.Core.Logging;
 using Coverlet.Core.Samples.Tests;
 using Microsoft.CodeAnalysis;
@@ -231,5 +232,26 @@ public void TestInstrument_NetStandardAwareAssemblyResolver_FromFolder()
             // We check if final netstandard.dll resolved is local folder one and not "official" netstandard.dll
             Assert.Equal(Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "netstandard.dll"), Path.GetFullPath(resolved.MainModule.FileName));
         }
+
+        [Fact]
+        public void SkipEmbeddedPpdbWithoutLocalSource()
+        {
+            string xunitDll = Directory.GetFiles(Directory.GetCurrentDirectory(), "xunit.*.dll").First();
+            var loggerMock = new Mock<ILogger>();
+            Instrumenter instrumenter = new Instrumenter(xunitDll, "_xunit_instrumented", Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>(), false, loggerMock.Object);
+            Assert.True(InstrumentationHelper.HasPdb(xunitDll, out bool embedded));
+            Assert.True(embedded);
+            Assert.False(instrumenter.CanInstrument());
+            loggerMock.Verify(l => l.LogWarning(It.IsAny<string>()));
+
+            // Default case
+            string coverletCoreDll = Directory.GetFiles(Directory.GetCurrentDirectory(), "coverlet.core.dll").First();
+            instrumenter = new Instrumenter(coverletCoreDll, "_coverlet_core_instrumented", Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>(), false, loggerMock.Object);
+            Assert.True(InstrumentationHelper.HasPdb(coverletCoreDll, out embedded));
+            Assert.False(embedded);
+            Assert.True(instrumenter.CanInstrument());
+            loggerMock.VerifyNoOtherCalls();
+        }
+
     }
 }