From 0e6520fe9e66944137112a578a41bfca274ab6fb Mon Sep 17 00:00:00 2001 From: Volodymyr Shkolka Date: Sat, 20 Apr 2024 03:49:30 +0300 Subject: [PATCH] Reworked modules filtering process (#1645) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Reworked modules filtering process * Added case insitivity * refactoring + coverage * nit * update change log --------- Co-authored-by: David Müller --- Documentation/Changelog.md | 1 + .../Abstractions/IInstrumentationHelper.cs | 4 +- src/coverlet.core/Coverage.cs | 13 ++- .../Helpers/InstrumentationHelper.cs | 83 +++++++++---------- .../Helpers/InstrumentationHelperTests.cs | 53 ++++++------ 5 files changed, 78 insertions(+), 76 deletions(-) diff --git a/Documentation/Changelog.md b/Documentation/Changelog.md index 67fea02d4..e45171b6f 100644 --- a/Documentation/Changelog.md +++ b/Documentation/Changelog.md @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased ### Fixed +- Fix slow modules filtering process [#1646](https://github.com/coverlet-coverage/coverlet/issues/1646) by https://github.com/BlackGad - Fix incorrect coverage await using in generic method [#1490](https://github.com/coverlet-coverage/coverlet/issues/1490) ## Release date 2024-03-13 diff --git a/src/coverlet.core/Abstractions/IInstrumentationHelper.cs b/src/coverlet.core/Abstractions/IInstrumentationHelper.cs index fee916509..d363fab63 100644 --- a/src/coverlet.core/Abstractions/IInstrumentationHelper.cs +++ b/src/coverlet.core/Abstractions/IInstrumentationHelper.cs @@ -1,6 +1,7 @@ // Copyright (c) Toni Solarin-Sodara // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System.Collections.Generic; using Coverlet.Core.Enums; namespace Coverlet.Core.Abstractions @@ -11,8 +12,7 @@ internal interface IInstrumentationHelper void DeleteHitsFile(string path); string[] GetCoverableModules(string module, string[] directories, bool includeTestAssembly); bool HasPdb(string module, out bool embedded); - bool IsModuleExcluded(string module, string[] excludeFilters); - bool IsModuleIncluded(string module, string[] includeFilters); + IEnumerable SelectModules(IEnumerable modules, string[] includeFilters, string[] excludeFilters); bool IsValidFilterExpression(string filter); bool IsTypeExcluded(string module, string type, string[] excludeFilters); bool IsTypeIncluded(string module, string type, string[] includeFilters); diff --git a/src/coverlet.core/Coverage.cs b/src/coverlet.core/Coverage.cs index e057a09bb..07c0297fa 100644 --- a/src/coverlet.core/Coverage.cs +++ b/src/coverlet.core/Coverage.cs @@ -107,15 +107,14 @@ public CoveragePrepareResult PrepareModules() _parameters.ExcludeFilters = _parameters.ExcludeFilters?.Where(f => _instrumentationHelper.IsValidFilterExpression(f)).ToArray(); _parameters.IncludeFilters = _parameters.IncludeFilters?.Where(f => _instrumentationHelper.IsValidFilterExpression(f)).ToArray(); - foreach (string module in modules) + IReadOnlyList validModules = _instrumentationHelper.SelectModules(modules, _parameters.IncludeFilters, _parameters.ExcludeFilters).ToList(); + foreach (var excludedModule in modules.Except(validModules)) { - if (_instrumentationHelper.IsModuleExcluded(module, _parameters.ExcludeFilters) || - !_instrumentationHelper.IsModuleIncluded(module, _parameters.IncludeFilters)) - { - _logger.LogVerbose($"Excluded module: '{module}'"); - continue; - } + _logger.LogVerbose($"Excluded module: '{excludedModule}'"); + } + foreach (string module in validModules) + { var instrumenter = new Instrumenter(module, Identifier, _parameters, diff --git a/src/coverlet.core/Helpers/InstrumentationHelper.cs b/src/coverlet.core/Helpers/InstrumentationHelper.cs index f6a35f73a..0c271c8ec 100644 --- a/src/coverlet.core/Helpers/InstrumentationHelper.cs +++ b/src/coverlet.core/Helpers/InstrumentationHelper.cs @@ -339,63 +339,60 @@ public bool IsValidFilterExpression(string filter) return true; } - public bool IsModuleExcluded(string module, string[] excludeFilters) + public IEnumerable SelectModules(IEnumerable modules, string[] includeFilters, string[] excludeFilters) { - if (excludeFilters == null || excludeFilters.Length == 0) - return false; + const char escapeSymbol = '!'; + ILookup modulesLookup = modules.Where(x => x != null) + .ToLookup(x => $"{escapeSymbol}{Path.GetFileNameWithoutExtension(x)}{escapeSymbol}"); - module = Path.GetFileNameWithoutExtension(module); - if (module == null) - return false; + string moduleKeys = string.Join(Environment.NewLine, modulesLookup.Select(x => x.Key)); + string includedModuleKeys = GetModuleKeysForIncludeFilters(includeFilters, escapeSymbol, moduleKeys); + string excludedModuleKeys = GetModuleKeysForExcludeFilters(excludeFilters, escapeSymbol, includedModuleKeys); - foreach (string filter in excludeFilters) - { -#pragma warning disable IDE0057 // Use range operator - string typePattern = filter.Substring(filter.IndexOf(']') + 1); - - if (typePattern != "*") - continue; + IEnumerable moduleKeysToInclude = includedModuleKeys + .Split(new[] { Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries) + .Except(excludedModuleKeys.Split(new[] { Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries)); - string modulePattern = filter.Substring(1, filter.IndexOf(']') - 1); -#pragma warning restore IDE0057 // Use range operator - modulePattern = WildcardToRegex(modulePattern); - - var regex = new Regex(modulePattern, s_regexOptions, TimeSpan.FromSeconds(10)); - - if (regex.IsMatch(module)) - return true; - } - - return false; + return moduleKeysToInclude.SelectMany(x => modulesLookup[x]); } - public bool IsModuleIncluded(string module, string[] includeFilters) + private string GetModuleKeysForIncludeFilters(IEnumerable filters, char escapeSymbol, string moduleKeys) { - if (includeFilters == null || includeFilters.Length == 0) - return true; + string[] validFilters = GetValidFilters(filters); - module = Path.GetFileNameWithoutExtension(module); - if (module == null) - return false; + return !validFilters.Any() ? moduleKeys : GetModuleKeysForValidFilters(escapeSymbol, moduleKeys, validFilters); + } - foreach (string filter in includeFilters) - { -#pragma warning disable IDE0057 // Use range operator - string modulePattern = filter.Substring(1, filter.IndexOf(']') - 1); -#pragma warning restore IDE0057 // Use range operator + private string GetModuleKeysForExcludeFilters(IEnumerable filters, char escapeSymbol, string moduleKeys) + { + string[] validFilters = GetValidFilters(filters); - if (modulePattern == "*") - return true; + return !validFilters.Any() ? string.Empty : GetModuleKeysForValidFilters(escapeSymbol, moduleKeys, validFilters); + } - modulePattern = WildcardToRegex(modulePattern); + private static string GetModuleKeysForValidFilters(char escapeSymbol, string moduleKeys, string[] validFilters) + { + string pattern = CreateRegexPattern(validFilters, escapeSymbol); + IEnumerable matches = Regex.Matches(moduleKeys, pattern, RegexOptions.IgnoreCase).Cast(); - var regex = new Regex(modulePattern, s_regexOptions, TimeSpan.FromSeconds(10)); + return string.Join( + Environment.NewLine, + matches.Where(x => x.Success).Select(x => x.Groups[0].Value)); + } - if (regex.IsMatch(module)) - return true; - } + private string[] GetValidFilters(IEnumerable filters) + { + return (filters ?? Array.Empty()) + .Where(IsValidFilterExpression) + .Where(x => x.EndsWith("*")) + .ToArray(); + } - return false; + private static string CreateRegexPattern(IEnumerable filters, char escapeSymbol) + { + IEnumerable regexPatterns = filters.Select(x => + $"{escapeSymbol}{WildcardToRegex(x.Substring(1, x.IndexOf(']') - 1)).Trim('^', '$')}{escapeSymbol}"); + return string.Join("|", regexPatterns); } public bool IsTypeExcluded(string module, string type, string[] excludeFilters) diff --git a/test/coverlet.core.tests/Helpers/InstrumentationHelperTests.cs b/test/coverlet.core.tests/Helpers/InstrumentationHelperTests.cs index 7996f9761..91edf8589 100644 --- a/test/coverlet.core.tests/Helpers/InstrumentationHelperTests.cs +++ b/test/coverlet.core.tests/Helpers/InstrumentationHelperTests.cs @@ -124,6 +124,10 @@ public void TestIsValidFilterExpression() Assert.False(_instrumentationHelper.IsValidFilterExpression("[-]*")); Assert.False(_instrumentationHelper.IsValidFilterExpression("*")); Assert.False(_instrumentationHelper.IsValidFilterExpression("][")); + Assert.False(_instrumentationHelper.IsValidFilterExpression("[")); + Assert.False(_instrumentationHelper.IsValidFilterExpression("[assembly][*")); + Assert.False(_instrumentationHelper.IsValidFilterExpression("[assembly]*]")); + Assert.False(_instrumentationHelper.IsValidFilterExpression("[]")); Assert.False(_instrumentationHelper.IsValidFilterExpression(null)); } @@ -138,19 +142,12 @@ public void TestDeleteHitsFile() } [Fact] - public void TestIsModuleExcludedWithoutFilter() + public void TestSelectModulesWithoutIncludeAndExcludedFilters() { - bool result = _instrumentationHelper.IsModuleExcluded("Module.dll", new string[0]); + string[] modules = new [] {"Module.dll"}; + IEnumerable result = _instrumentationHelper.SelectModules(modules, new string[0], new string[0]); - Assert.False(result); - } - - [Fact] - public void TestIsModuleIncludedWithoutFilter() - { - bool result = _instrumentationHelper.IsModuleIncluded("Module.dll", new string[0]); - - Assert.True(result); + Assert.Equal(modules, result); } [Theory] @@ -158,41 +155,41 @@ public void TestIsModuleIncludedWithoutFilter() [InlineData("[Mismatch]*")] public void TestIsModuleExcludedWithSingleMismatchFilter(string filter) { - bool result = _instrumentationHelper.IsModuleExcluded("Module.dll", new[] { filter }); + string[] modules = new [] {"Module.dll"}; + IEnumerable result = _instrumentationHelper.SelectModules(modules, new string[0], new[] {filter}); - Assert.False(result); + Assert.Equal(modules, result); } [Fact] public void TestIsModuleIncludedWithSingleMismatchFilter() { - bool result = _instrumentationHelper.IsModuleIncluded("Module.dll", new[] { "[Mismatch]*" }); + string[] modules = new [] {"Module.dll"}; + IEnumerable result = _instrumentationHelper.SelectModules(modules, new[] { "[Mismatch]*" }, new string[0]); - Assert.False(result); + Assert.Empty(result); } [Theory] [MemberData(nameof(ValidModuleFilterData))] public void TestIsModuleExcludedAndIncludedWithFilter(string filter) { - bool result = _instrumentationHelper.IsModuleExcluded("Module.dll", new[] { filter }); - Assert.True(result); + string[] modules = new [] {"Module.dll"}; + IEnumerable result = _instrumentationHelper.SelectModules(modules, new[] { filter }, new[] { filter }); - result = _instrumentationHelper.IsModuleIncluded("Module.dll", new[] { filter }); - Assert.True(result); + Assert.Empty(result); } [Theory] [MemberData(nameof(ValidModuleFilterData))] public void TestIsModuleExcludedAndIncludedWithMatchingAndMismatchingFilter(string filter) { - string[] filters = new[] { "[Mismatch]*", filter, "[Mismatch]*" }; + string[] modules = new[] {"Module.dll"}; + string[] filters = new[] {"[Mismatch]*", filter, "[Mismatch]*"}; - bool result = _instrumentationHelper.IsModuleExcluded("Module.dll", filters); - Assert.True(result); + IEnumerable result = _instrumentationHelper.SelectModules(modules, filters, filters); - result = _instrumentationHelper.IsModuleIncluded("Module.dll", filters); - Assert.True(result); + Assert.Empty(result); } [Fact] @@ -305,6 +302,14 @@ public void TestIncludeDirectories() newDir2.Delete(true); } + [Theory] + [InlineData("g__LocalFunction|0_0", true)] + [InlineData("TestMethod", false)] + public void InstrumentationHelper_IsLocalMethod_ReturnsExpectedResult(string method, bool result) + { + Assert.Equal(_instrumentationHelper.IsLocalMethod(method), result); + } + public static IEnumerable ValidModuleFilterData => new List {