Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use diff engine #644

Merged
merged 9 commits into from Aug 4, 2020
Merged

Use diff engine #644

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3e69bfb
use diffengine
SimonCropp Jul 11, 2020
b167006
.
SimonCropp Jul 11, 2020
5e7ecdc
add obsoletes
SimonCropp Jul 12, 2020
1748da8
Update Shouldly.csproj
SimonCropp Jul 14, 2020
3682642
Merge branch 'master' into useDiffEngine
SimonCropp Jul 16, 2020
a6f0d88
Merge branch 'master' into useDiffEngine
SimonCropp Jul 21, 2020
0b4856c
Merge branch 'master' into useDiffEngine
SimonCropp Jul 21, 2020
c0510c5
Update ApprovePublicApi.ShouldlyApi.approved.cs
SimonCropp Jul 21, 2020
96b3fe0
Update ShouldMatchApprovedScenarios.cs
SimonCropp Jul 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
74 changes: 0 additions & 74 deletions src/Shouldly.Tests/ConventionTests/ApprovePublicApi.ShouldlyApi.approved.cs
View file
Open in desktop
Expand Up @@ -818,7 +818,6 @@ public static class ShouldlyConfiguration
public static double DefaultFloatingPointTolerance;
public static System.TimeSpan DefaultTaskTimeout;
public static System.Collections.Generic.List<string> CompareAsObjectTypes { get; }
public static Shouldly.Configuration.DiffToolConfiguration DiffTools { get; }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will existing code using Shouldly v3 know what to do upon upgrading to v4?

Copy link
Collaborator

@jnm2 jnm2 Jul 11, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible in principle to preserve Shouldly's existing API and delegate to DiffEngine for the implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will existing code using Shouldly v3 know what to do upon upgrading to v4?

I added obsoletes

Is it possible in principle to preserve Shouldly's existing API and delegate to DiffEngine for the implementation

i would prefer not to. reasons:

  • many of the diff APIs should not have been made public. eg KnownDoNotLaunchStrategies, DoNotLaunchWhenPlatformIsNotWindows
  • there is a mismatch between the APIs. for example shouldly is hard coded to expect that a single tool has the same args for different OS. this is not correct eg https://github.com/VerifyTests/DiffEngine/blob/master/docs/diff-tool.md#windows-settings so that cant be properly represented in the existing shouldly api. The may be other examples i have not found, and future changes in DiffEngine could result in more of a mismatch
  • most of the shouldly Diff api should be very rarely used required by people. for example given the tools supported by DiffEngine people should rarely need to add a custom tool
  • shouldly lack opinion on if settings should be personal or shared
  • the shouldly API docs would need be continually maintained

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have toyed with the idea of avoiding a nuget ref. submodules or a source package. but ultimately the docs was too difficult to replicate

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thoughts on the above? this PR changes enough that i am holding of on the next few things

Copy link
Collaborator

@jnm2 jnm2 Jul 14, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That answers my question, thanks. I was thinking it could ease the transition, but if we want the obsoleted APIs to continue working in v4, we should just leave the v3 impl intact. If we don't, I think I'm good with that with the caveat that I haven't used diffing so I don't have a good sense of what people might expect.

public static Shouldly.Configuration.ShouldMatchConfigurationBuilder ShouldMatchApprovedDefaults { get; }
public static System.IDisposable DisableSourceInErrors() { }
public static bool IsSourceDisabledInErrors() { }
Expand Down Expand Up @@ -847,48 +846,6 @@ public enum StringCompareShould
}
namespace Shouldly.Configuration
{
public class DiffTool
{
public DiffTool(string name, Shouldly.Configuration.DiffToolConfig config, Shouldly.Configuration.DiffTool.ArgumentGenerator argGenerator) { }
public string Name { get; }
public bool Exists() { }
public void Open(string receivedPath, string approvedPath, bool approvedExists) { }
public delegate string ArgumentGenerator(string received, string approved, bool approvedExists);
}
public class DiffToolConfig
{
public DiffToolConfig() { }
public string? LinuxPath { get; set; }
public string? MacPath { get; set; }
public string? WindowsPath { get; set; }
public string? ResolvePath() { }
}
public class DiffToolConfiguration
{
public DiffToolConfiguration() { }
public Shouldly.Configuration.KnownDiffTools KnownDiffTools { get; }
public Shouldly.Configuration.KnownDoNotLaunchStrategies KnownDoNotLaunchStrategies { get; }
public void AddDoNotLaunchStrategy(Shouldly.Configuration.IShouldNotLaunchDiffTool shouldNotlaunchStrategy) { }
public Shouldly.Configuration.DiffTool GetDiffTool() { }
public void RegisterDiffTool(Shouldly.Configuration.DiffTool diffTool) { }
public void SetDiffToolPriorities(params Shouldly.Configuration.DiffTool[] diffTools) { }
public bool ShouldOpenDiffTool() { }
}
public class DoNotLaunchWhenEnvVariableIsPresent : Shouldly.Configuration.IShouldNotLaunchDiffTool
{
public DoNotLaunchWhenEnvVariableIsPresent(string environmentalVariable) { }
public bool ShouldNotLaunch() { }
}
public class DoNotLaunchWhenPlatformIsNotWindows : Shouldly.Configuration.IShouldNotLaunchDiffTool
{
public DoNotLaunchWhenPlatformIsNotWindows() { }
public bool ShouldNotLaunch() { }
}
public class DoNotLaunchWhenTypeIsLoaded : Shouldly.Configuration.IShouldNotLaunchDiffTool
{
public DoNotLaunchWhenTypeIsLoaded(string typeName) { }
public bool ShouldNotLaunch() { }
}
public delegate string FilenameGenerator(Shouldly.Configuration.TestMethodInfo testMethodInfo, string? discriminator, string fileType, string fileExtension);
public class FindMethodUsingAttribute<T> : Shouldly.Configuration.ITestMethodFinder
where T : System.Attribute
Expand All @@ -902,41 +859,10 @@ public class FirstNonShouldlyMethodFinder : Shouldly.Configuration.ITestMethodFi
public int Offset { get; set; }
public Shouldly.Configuration.TestMethodInfo GetTestMethodInfo(System.Diagnostics.StackTrace stackTrace, int startAt = 0) { }
}
public interface IShouldNotLaunchDiffTool
{
bool ShouldNotLaunch();
}
public interface ITestMethodFinder
{
Shouldly.Configuration.TestMethodInfo GetTestMethodInfo(System.Diagnostics.StackTrace stackTrace, int startAt = 0);
}
public class KnownDiffTools
{
public readonly Shouldly.Configuration.DiffTool BeyondCompare3;
public readonly Shouldly.Configuration.DiffTool BeyondCompare4;
public readonly Shouldly.Configuration.DiffTool CodeCompare;
public readonly Shouldly.Configuration.DiffTool CurrentVisualStudio;
public readonly Shouldly.Configuration.DiffTool KDiff3;
public readonly Shouldly.Configuration.DiffTool P4Merge;
public readonly Shouldly.Configuration.DiffTool TortoiseGitMerge;
public readonly Shouldly.Configuration.DiffTool VisualStudioCode;
public readonly Shouldly.Configuration.DiffTool WinMerge;
public KnownDiffTools() { }
public static Shouldly.Configuration.KnownDiffTools Instance { get; }
}
public class KnownDoNotLaunchStrategies
{
public readonly Shouldly.Configuration.IShouldNotLaunchDiffTool AppVeyor;
public readonly Shouldly.Configuration.IShouldNotLaunchDiffTool GitLabCI;
public readonly Shouldly.Configuration.IShouldNotLaunchDiffTool Jenkins;
public readonly Shouldly.Configuration.IShouldNotLaunchDiffTool MyGet;
public readonly Shouldly.Configuration.IShouldNotLaunchDiffTool NCrunch;
public readonly Shouldly.Configuration.IShouldNotLaunchDiffTool TeamCity;
public readonly Shouldly.Configuration.IShouldNotLaunchDiffTool TravisCI;
public readonly Shouldly.Configuration.IShouldNotLaunchDiffTool VSTS;
public readonly Shouldly.Configuration.IShouldNotLaunchDiffTool VisualStudioLiveUnitTesting;
public KnownDoNotLaunchStrategies() { }
}
public class ShouldMatchConfiguration
{
public ShouldMatchConfiguration() { }
Expand Down
23 changes: 0 additions & 23 deletions src/Shouldly.Tests/ShouldMatchApproved/ShouldMatchApprovedScenarios.cs
View file
Open in desktop
Expand Up @@ -102,29 +102,6 @@ but was
_scrubber);
}


[Fact]
public void NoDiffToolsFound()
{
var diffTools = ShouldlyConfiguration.DiffTools.GetType()
.GetField("_diffTools", BindingFlags.Instance | BindingFlags.NonPublic)!;
var diffToolsCollection = (List<DiffTool>)diffTools.GetValue(ShouldlyConfiguration.DiffTools)!;
var currentDiffTools = new List<DiffTool>(diffToolsCollection);

try
{
diffToolsCollection.Clear();
var ex = Should.Throw<ShouldAssertException>(() => ShouldlyConfiguration.DiffTools.GetDiffTool());
ex.Message.ShouldBe(@"Cannot find a difftool to use, please open an issue or a PR to add support for your difftool.

In the meantime use 'ShouldlyConfiguration.DiffTools.RegisterDiffTool()' to add your own");
}
finally
{
diffToolsCollection.AddRange(currentDiffTools);
}
}

[Fact]
public void IgnoresLineEndingsByDefault()
{
Expand Down
6 changes: 0 additions & 6 deletions src/Shouldly.sln
View file
Open in desktop
Expand Up @@ -21,8 +21,6 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Shouldly", "Shouldly\Should
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Shouldly.Tests", "Shouldly.Tests\Shouldly.Tests.csproj", "{70F31200-AE2B-42A9-9C56-200EED6235BD}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "TestDiffTools", "TestDiffTools\TestDiffTools.csproj", "{4BB4C03C-EACF-4778-A434-60E1EF90EC9A}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "DocumentationExamples", "DocumentationExamples\DocumentationExamples.csproj", "{D5990C0D-729B-4A88-9783-36F6D1898E38}"
EndProject
Global
Expand All @@ -39,10 +37,6 @@ Global
{70F31200-AE2B-42A9-9C56-200EED6235BD}.Debug|Any CPU.Build.0 = Debug|Any CPU
{70F31200-AE2B-42A9-9C56-200EED6235BD}.Release|Any CPU.ActiveCfg = Release|Any CPU
{70F31200-AE2B-42A9-9C56-200EED6235BD}.Release|Any CPU.Build.0 = Release|Any CPU
{4BB4C03C-EACF-4778-A434-60E1EF90EC9A}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{4BB4C03C-EACF-4778-A434-60E1EF90EC9A}.Debug|Any CPU.Build.0 = Debug|Any CPU
{4BB4C03C-EACF-4778-A434-60E1EF90EC9A}.Release|Any CPU.ActiveCfg = Release|Any CPU
{4BB4C03C-EACF-4778-A434-60E1EF90EC9A}.Release|Any CPU.Build.0 = Release|Any CPU
{D5990C0D-729B-4A88-9783-36F6D1898E38}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{D5990C0D-729B-4A88-9783-36F6D1898E38}.Debug|Any CPU.Build.0 = Debug|Any CPU
{D5990C0D-729B-4A88-9783-36F6D1898E38}.Release|Any CPU.ActiveCfg = Release|Any CPU
Expand Down
127 changes: 0 additions & 127 deletions src/Shouldly/Configuration/DiffTool.cs
View file
Open in desktop

This file was deleted.

71 changes: 0 additions & 71 deletions src/Shouldly/Configuration/DiffToolConfiguration.cs
View file
Open in desktop

This file was deleted.

23 changes: 0 additions & 23 deletions src/Shouldly/Configuration/DoNotLaunchWhenEnvVariableIsPresent.cs
View file
Open in desktop

This file was deleted.

10 changes: 0 additions & 10 deletions src/Shouldly/Configuration/DoNotLaunchWhenPlatformIsNotWindows.cs
View file
Open in desktop

This file was deleted.