Decoupling file excludability from mutant spans #2645

psfinaki · 2023-08-18T20:52:44Z

Phew... Okay so this is a somewhat bigger steps towards the F# support.

Motivation

F# has its own understanding of text documents (btw that's sad). Roslyn (in this context C#) generally counts all characters from zero whereas F# generally uses lines and columns. Of course one is convertible to the other, it's more about what's native to the compiler. Type-wise, it's Microsoft.CodeAnalysis.Text.TextSpan versus FSharp.Compiler.Text.Range.

Sooner or later we would need native span arithmetics for mutants hence one approach is to create a definition of mutant span on the user level (= configuration level) and then translate them to compiler definitions of spans based on the file type.

Implementation

This PR

Makes FilePattern language-agnostic
Splits the concept of excludability to its own place since that concept has some usage on its own
Adds some span (range) arithmetics for F#
Hopefully brings some collateral improvements

TO DO

tests
basic parameter validation for new types
docs
see comments in the PR

src/Stryker.Core/Stryker.Core.UnitTest/DiffProviders/GitDiffProviderTests.cs

src/Stryker.Core/Stryker.Core.UnitTest/MutantFilters/FilePatternMutantFilterTests.cs

src/Stryker.Core/Stryker.Core.UnitTest/Options/FilePatternTests.cs

src/Stryker.Core/Stryker.Core/MutantFilters/FilePatternMutantFilter.cs

src/Stryker.Core/Stryker.Core/Options/StrykerOptions.cs

src/Stryker.Core/Stryker.Core/ProjectComponents/ExcludableProjectComponent.cs

psfinaki · 2023-08-18T21:29:19Z

src/Stryker.Core/Stryker.Core/ProjectComponents/Fsharp/FsharpFileLeaf.cs

+            return pattern.MutantSpans.Any(span => RangeModule.rangeContainsRange(FromMutantSpan(span), range));
+        }
+
+        public override IEnumerable<Range> Reduce(IEnumerable<Range> spans)


Actually much will be probably taken from the TextSpanHelper, eventually using the same model as is done for span matching.

Will the TextSpanHelper be the abstraction for span and range?

Well as of now TextSpanHelper is a set of extensions on top of TextSpan (C#) whereas RangeHelper is a set of extensions on top of Range (F#). But let me see if I can refactor it a bit, there is some common code to extract.

psfinaki · 2023-08-18T21:31:55Z

src/Stryker.Core/Stryker.Core/ProjectComponents/SimpleFileLeaf.cs

+
+namespace Stryker.Core.ProjectComponents
+{
+    public class SimpleFileLeaf: ProjectComponent, IReadOnlyFileLeaf


This is really needed only GitDiffProvider. Although it's useful in tests also.

Now, I guess this could be somehow inserted in the tree of abstractions here since there is some logic duplication with ExcludableProjectComponent. But my brain already hurts from abstractions here and the duplication is rather small anyway.

I'm not sure what this class is doing.. Is this a non-language-specific file? Should this be an abstract class?

Correct, this is a non-language specific file. It's initiated in the GitDiffProvider where we indeed don't care about the code:

private void RemoveFilteredOutFiles(DiffResult diffResult) { foreach (var glob in new SimpleFileLeaf(_options.DiffIgnoreChanges).Patterns.Select(d => d.Glob)) { diffResult.ChangedSourceFiles = diffResult.ChangedSourceFiles.Where(diffResultFile => !glob.IsMatch(diffResultFile)).ToList(); diffResult.ChangedTestFiles = diffResult.ChangedTestFiles.Where(diffResultFile => !glob.IsMatch(diffResultFile)).ToList(); } }

psfinaki · 2023-08-18T21:33:10Z

src/Stryker.Core/Stryker.Core/Reporters/ClearTextReporter.cs

@@ -78,7 +78,13 @@ private void DisplayComponent(IReadOnlyProjectComponent inputComponent, Table ta

            var mutationScore = inputComponent.GetMutationScore();

-            if (inputComponent.IsComponentExcluded(_options.Mutate))
+            var isExcluded = inputComponent switch


Trying to understand this... is this correct? Can a folder be excluded in the context of clear text reporters?

Yes, with globbing it's possible to exclude a whole folder. So it will be displayed as excluded. However, now that you mention this, we moved from exclude to ignore as the preferred term. So maybe we need to display [Ignored] instead of [Excluded].

psfinaki · 2023-08-18T21:35:01Z

Unit tests pass, there are two integration tests failing, I will look into them.

Maintainers - please, before I continue on this. Do you think this is a reasonable design change? Is the current change something you approve "in principle"? :)

psfinaki · 2023-09-01T14:59:39Z

Ping @richardwerkman @rouke-broersma @dupdob :)

richardwerkman

Do you think this is a reasonable design change? Is the current change something you approve "in principle"? :)

I think this is generally going in the right direction. I have some questions regarding the abstractions, but those are kind of a mess anyway.

We could also choose to not implement some of the deeper features in F#, like ignoring part of a file. If this makes the transition to F# easier. We can always implement it later

richardwerkman · 2023-09-01T16:58:40Z

src/Stryker.Core/Stryker.Core/Reporters/ClearTextReporter.cs

@@ -78,7 +78,13 @@ private void DisplayComponent(IReadOnlyProjectComponent inputComponent, Table ta

            var mutationScore = inputComponent.GetMutationScore();

-            if (inputComponent.IsComponentExcluded(_options.Mutate))
+            var isExcluded = inputComponent switch


Yes, with globbing it's possible to exclude a whole folder. So it will be displayed as excluded. However, now that you mention this, we moved from exclude to ignore as the preferred term. So maybe we need to display [Ignored] instead of [Excluded].

richardwerkman · 2023-09-01T17:00:11Z

src/Stryker.Core/Stryker.Core/ProjectComponents/Fsharp/FsharpFileLeaf.cs

+            return pattern.MutantSpans.Any(span => RangeModule.rangeContainsRange(FromMutantSpan(span), range));
+        }
+
+        public override IEnumerable<Range> Reduce(IEnumerable<Range> spans)


Will the TextSpanHelper be the abstraction for span and range?

richardwerkman · 2023-09-01T17:06:43Z

src/Stryker.Core/Stryker.Core/ProjectComponents/SimpleFileLeaf.cs

+
+namespace Stryker.Core.ProjectComponents
+{
+    public class SimpleFileLeaf: ProjectComponent, IReadOnlyFileLeaf


I'm not sure what this class is doing.. Is this a non-language-specific file? Should this be an abstract class?

richardwerkman · 2023-09-01T17:09:15Z

src/Stryker.Core/Stryker.Core/ProjectComponents/ExcludableString.cs

@@ -0,0 +1,17 @@
+namespace Stryker.Core.ProjectComponents
+{
+    public class ExcludableString


What does this class do? And is this cSharp specific?

Well, this is now basically just a non-language-specific construct to represent the idea that we can include or exclude files in some code flows. Distinguish Person.cs vs !Person.cs or Person.fs vs Person.fs. E.g. DiffIgnoreChangesInput don't need info about spans as opposed to MutateInput.

psfinaki · 2023-09-21T18:39:29Z

Okay sorry for the delay, I am back on this. Now tests pass, int tests pass, good stuff. Let me see if I can refactor things a bit and add some extra tests, I will let you know then :)

psfinaki · 2023-09-23T17:22:13Z

Okay, so this one is getting quite big and I promised small PRs :)

I extracted the changes to the following PR:

Let's address those ones first and then see what's left here. Meanwhile converting to draft again.

psfinaki marked this pull request as draft August 18, 2023 20:52

psfinaki commented Aug 18, 2023

View reviewed changes

richardwerkman reviewed Sep 1, 2023

View reviewed changes

psfinaki added 2 commits September 21, 2023 16:58

Decoupling file excludability from mutant spans

754f469

Fix

8581c9b

psfinaki force-pushed the fsharp-20 branch from 9e6ee57 to 8581c9b Compare September 21, 2023 14:58

psfinaki added 2 commits September 21, 2023 17:13

up

0e7674f

up

8893c12

psfinaki marked this pull request as ready for review September 21, 2023 18:38

psfinaki added 3 commits September 22, 2023 16:54

A bunch of F# range math + range reduction

c5a0720

moar range math

6c0f8db

up

513cb37

This was referenced Sep 23, 2023

chore: Passing Stryker options to F# project components #2680

Merged

chore: Breaking down FilePattern #2681

Merged

chore: F# range math #2679

Merged

psfinaki marked this pull request as draft September 23, 2023 17:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Decoupling file excludability from mutant spans #2645

Decoupling file excludability from mutant spans #2645

psfinaki commented Aug 18, 2023 •

edited

psfinaki Aug 18, 2023

richardwerkman Sep 1, 2023

psfinaki Sep 21, 2023

psfinaki Aug 18, 2023

richardwerkman Sep 1, 2023

psfinaki Sep 21, 2023

psfinaki Aug 18, 2023

richardwerkman Sep 1, 2023

psfinaki commented Aug 18, 2023

psfinaki commented Sep 1, 2023

richardwerkman left a comment

richardwerkman Sep 1, 2023

richardwerkman Sep 1, 2023

richardwerkman Sep 1, 2023

richardwerkman Sep 1, 2023

psfinaki Sep 21, 2023

psfinaki commented Sep 21, 2023

psfinaki commented Sep 23, 2023

Decoupling file excludability from mutant spans #2645

Are you sure you want to change the base?

Decoupling file excludability from mutant spans #2645

Conversation

psfinaki commented Aug 18, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

psfinaki commented Aug 18, 2023

psfinaki commented Sep 1, 2023

richardwerkman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

psfinaki commented Sep 21, 2023

psfinaki commented Sep 23, 2023

psfinaki commented Aug 18, 2023 •

edited