From f5bda1e4992522c2882e13913e94b82d3d656d20 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Sat, 1 Feb 2025 17:37:24 +0000 Subject: [PATCH] Fix [Explicit] attribute not being honoured when running from IDE Test Explorers (#1739) --- .../Framework/TUnitServiceProvider.cs | 4 +-- TUnit.Engine/Models/GroupedTests.cs | 2 +- .../Services/ExplicitFilterService.cs | 29 ----------------- TUnit.Engine/Services/SingleTestExecutor.cs | 6 ---- TUnit.Engine/Services/TUnitTestDiscoverer.cs | 4 +-- TUnit.Engine/Services/TestFilterService.cs | 32 +++++++++++++++---- TUnit.Engine/Services/TestGrouper.cs | 2 +- docs/docs/tutorial-extras/explicit.md | 9 ++---- 8 files changed, 34 insertions(+), 54 deletions(-) delete mode 100644 TUnit.Engine/Services/ExplicitFilterService.cs diff --git a/TUnit.Engine/Framework/TUnitServiceProvider.cs b/TUnit.Engine/Framework/TUnitServiceProvider.cs index 7107dad7c4..4b51d146aa 100644 --- a/TUnit.Engine/Framework/TUnitServiceProvider.cs +++ b/TUnit.Engine/Framework/TUnitServiceProvider.cs @@ -96,14 +96,12 @@ public TUnitServiceProvider(IExtension extension, Disposer = Register(new Disposer(Logger)); var testInvoker = Register(new TestInvoker(testHookOrchestrator, Logger, Disposer)); - var explicitFilterService = Register(new ExplicitFilterService()); var parallelLimitProvider = Register(new ParallelLimitLockProvider()); // TODO Register(new HookMessagePublisher(extension, messageBus)); - var singleTestExecutor = Register(new SingleTestExecutor(extension, instanceTracker, testInvoker, - explicitFilterService, parallelLimitProvider, AssemblyHookOrchestrator, classHookOrchestrator, TestFinder, TUnitMessageBus, Logger, EngineCancellationToken, testRegistrar)); + var singleTestExecutor = Register(new SingleTestExecutor(extension, instanceTracker, testInvoker, parallelLimitProvider, AssemblyHookOrchestrator, classHookOrchestrator, TestFinder, TUnitMessageBus, Logger, EngineCancellationToken, testRegistrar)); TestsExecutor = Register(new TestsExecutor(singleTestExecutor, Logger, CommandLineOptions, EngineCancellationToken)); diff --git a/TUnit.Engine/Models/GroupedTests.cs b/TUnit.Engine/Models/GroupedTests.cs index dc122bf72a..53f4f09962 100644 --- a/TUnit.Engine/Models/GroupedTests.cs +++ b/TUnit.Engine/Models/GroupedTests.cs @@ -4,7 +4,7 @@ namespace TUnit.Engine.Models; internal record GroupedTests { - public required DiscoveredTest[] AllValidTests { get; init; } + public required IReadOnlyCollection AllValidTests { get; init; } public required PriorityQueue NotInParallel { get; init; } public required IDictionary> KeyedNotInParallel { get; init; } public required IList Parallel { get; init; } diff --git a/TUnit.Engine/Services/ExplicitFilterService.cs b/TUnit.Engine/Services/ExplicitFilterService.cs deleted file mode 100644 index 9159555354..0000000000 --- a/TUnit.Engine/Services/ExplicitFilterService.cs +++ /dev/null @@ -1,29 +0,0 @@ -using Microsoft.Testing.Platform.Requests; -using TUnit.Core; - -#pragma warning disable TPEXP - -namespace TUnit.Engine.Services; - -internal class ExplicitFilterService -{ - public bool CanRun(TestDetails testDetails, ITestExecutionFilter? filter) - { - if (!testDetails.Attributes.Any(x => x is ExplicitAttribute)) - { - // These tests don't have any ExplicitAttributes - return true; - } - - if (filter is null or NopFilter) - { - return false; - } - - // Filters have already done matching - And the filter is not null due to the above check - // So we've explicitly filtered these tests! - return true; - } - - -} \ No newline at end of file diff --git a/TUnit.Engine/Services/SingleTestExecutor.cs b/TUnit.Engine/Services/SingleTestExecutor.cs index e6d7237c29..b2681ad1aa 100644 --- a/TUnit.Engine/Services/SingleTestExecutor.cs +++ b/TUnit.Engine/Services/SingleTestExecutor.cs @@ -22,7 +22,6 @@ internal class SingleTestExecutor( IExtension extension, InstanceTracker instanceTracker, TestInvoker testInvoker, - ExplicitFilterService explicitFilterService, ParallelLimitLockProvider parallelLimitLockProvider, AssemblyHookOrchestrator assemblyHookOrchestrator, ClassHookOrchestrator classHookOrchestrator, @@ -79,11 +78,6 @@ private async Task ExecuteTestInternalAsync(DiscoveredTest test, ITestExecutionF await RegisterIfNotAlready(testContext); - if (!explicitFilterService.CanRun(test.TestDetails, filter)) - { - throw new SkipTestException("Test with ExplicitAttribute was not explicitly run."); - } - if (testContext.SkipReason != null) { throw new SkipTestException(testContext.SkipReason); diff --git a/TUnit.Engine/Services/TUnitTestDiscoverer.cs b/TUnit.Engine/Services/TUnitTestDiscoverer.cs index a7f65b6908..4733bcddcd 100644 --- a/TUnit.Engine/Services/TUnitTestDiscoverer.cs +++ b/TUnit.Engine/Services/TUnitTestDiscoverer.cs @@ -36,9 +36,9 @@ public async Task FilterTests(ExecuteRequestContext context, strin var executionRequest = context.Request as TestExecutionRequest; - var filteredTests = testFilterService.FilterTests(executionRequest?.Filter, allDiscoveredTests).ToArray(); + var filteredTests = testFilterService.FilterTests(executionRequest, allDiscoveredTests); - await logger.LogTraceAsync($"Found {filteredTests.Length} tests after filtering."); + await logger.LogTraceAsync($"Found {filteredTests.Count} tests after filtering."); var organisedTests = testGrouper.OrganiseTests(filteredTests); diff --git a/TUnit.Engine/Services/TestFilterService.cs b/TUnit.Engine/Services/TestFilterService.cs index eea9a43047..0b7f762211 100644 --- a/TUnit.Engine/Services/TestFilterService.cs +++ b/TUnit.Engine/Services/TestFilterService.cs @@ -1,4 +1,6 @@ -using Microsoft.Testing.Platform.Extensions.Messages; +#pragma warning disable TPEXP + +using Microsoft.Testing.Platform.Extensions.Messages; using Microsoft.Testing.Platform.Logging; using Microsoft.Testing.Platform.Requests; using TUnit.Core; @@ -9,20 +11,38 @@ internal class TestFilterService(ILoggerFactory loggerFactory) { private readonly ILogger _logger = loggerFactory.CreateLogger(); - public IEnumerable FilterTests(ITestExecutionFilter? testExecutionFilter, IEnumerable testNodes) + public IReadOnlyCollection FilterTests(TestExecutionRequest? testExecutionRequest, IReadOnlyCollection testNodes) { -#pragma warning disable TPEXP + var testExecutionFilter = testExecutionRequest?.Filter; + if (testExecutionFilter is null or NopFilter) -#pragma warning restore TPEXP { _logger.LogTrace("No test filter found."); - + + if (testExecutionRequest is RunTestExecutionRequest) + { + return testNodes + .Where(x => !x.TestDetails.Attributes.OfType().Any()) + .ToArray(); + } + return testNodes; } _logger.LogTrace($"Test filter is: {testExecutionFilter.GetType().Name}"); - return testNodes.Where(x => MatchesTest(testExecutionFilter, x)); + var filteredTests = testNodes.Where(x => MatchesTest(testExecutionFilter, x)).ToArray(); + + var testsWithExplicitAttributeCount = filteredTests.Count(x => x.TestDetails.Attributes.OfType().Any()); + + if (testsWithExplicitAttributeCount > 0 && testsWithExplicitAttributeCount < filteredTests.Length) + { + return testNodes + .Where(x => !x.TestDetails.Attributes.OfType().Any()) + .ToArray(); + } + + return filteredTests; } public bool MatchesTest(ITestExecutionFilter? testExecutionFilter, DiscoveredTest discoveredTest) diff --git a/TUnit.Engine/Services/TestGrouper.cs b/TUnit.Engine/Services/TestGrouper.cs index b195f53c31..8d6ee78c4f 100644 --- a/TUnit.Engine/Services/TestGrouper.cs +++ b/TUnit.Engine/Services/TestGrouper.cs @@ -6,7 +6,7 @@ namespace TUnit.Engine.Services; internal class TestGrouper { - public GroupedTests OrganiseTests(DiscoveredTest[] testCases) + public GroupedTests OrganiseTests(IReadOnlyCollection testCases) { var notInParallel = new PriorityQueue(); var keyedNotInParallel = new ConcurrentDictionary>(); diff --git a/docs/docs/tutorial-extras/explicit.md b/docs/docs/tutorial-extras/explicit.md index 79a08d0346..3f025a7087 100644 --- a/docs/docs/tutorial-extras/explicit.md +++ b/docs/docs/tutorial-extras/explicit.md @@ -8,14 +8,11 @@ If you want a test to only be run explicitly (and not part of all general tests) This can be added to a test method or a test class. -If added to a test method, then all tests generated from that test method (different test cases generated from different test data) will only be run if that method has been explicitly run. -If added to a test class, then all tests generated within that class will only be run if only tests within that class have been explicitly run. +A test is considered 'explicitly' run when all filtered tests have an explicit attribute on them. -A test is considered 'explicitly' run when: -- A test filter was used and matches your test - e.g. `dotnet run --treenode-filter "/*/*/*/*"` -- You ran your test from within the test explorer in your IDE specifically +That means that you could run all tests in a class with an `[Explicit]` attribute. Or you could run a single method with an `[Explicit]` attribute. But if you try to run a mix of explicit and non-explicit tests, then the ones with an `[Explicit]` attribute will be excluded from the run. +This can be useful for 'Tests' that make sense in a local environment, and maybe not part of your CI builds. Or they could be helpers that ping things to warm them up, and by making them explicit tests, they are easily runnable, but don't affect your overall test suite. ```csharp using TUnit.Core;