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

Add support for deterministic builds #883

Merged
merged 11 commits into from
Apr 12, 2023
Merged

Add support for deterministic builds #883

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
591fb21
Add support for deterministic builds
slang25 Mar 17, 2023
c0dceda
Approve Public API change
slang25 Mar 17, 2023
03a1507
Refactor for clarity
slang25 Mar 18, 2023
9c7152f
Fix minor error
slang25 Mar 18, 2023
14c1b0c
Enable deterministic build of Shouldly itself
slang25 Mar 18, 2023
0323ecc
Update SourceCodeTextGetter.cs
slang25 Mar 20, 2023
e0050a5
Merge branch 'master' into support-deterministic-builds
SimonCropp Apr 1, 2023
30d9dee
Merge branch 'master' into support-deterministic-builds
slang25 Apr 2, 2023
70252b1
Use environment variable instead of config for source root
slang25 Apr 7, 2023
7774682
Try using PathMap for source roots
slang25 Apr 9, 2023
f2ac390
Skip when SHOULDLY_SOURCE_PATH_MAP already set
slang25 Apr 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/DeterministicTests/DeterministicTests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
<PackageReference Include="Xunit" Version="2.4.2" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.4.5" />
<ProjectReference Include="..\Shouldly\Shouldly.csproj" />
<PackageReference Include="DotNet.ReproducibleBuilds" Version="1.1.1">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
</PackageReference>
</ItemGroup>
<Import Project="..\Shouldly\build.props" />
</Project>
2 changes: 0 additions & 2 deletions src/DocumentationExamples/DocumentationExamples.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

<PropertyGroup>
<TargetFramework>net7.0</TargetFramework>
<DebugType>full</DebugType>
</PropertyGroup>

<ItemGroup>
Expand All @@ -16,6 +15,5 @@
<PackageReference Include="PublicApiGenerator" Version="10.3.0" />
<PackageDownload Include="Microsoft.NETCore.App.Ref" Version="[3.1.0]" />
</ItemGroup>
<Import Project="..\Shouldly\build.props" />

</Project>
1 change: 1 addition & 0 deletions src/Shouldly.Tests/ConventionTests/ApprovePublicApi.ShouldlyApi.approved.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -920,6 +920,7 @@ public static class ShouldlyConfiguration
public static System.TimeSpan DefaultTaskTimeout;
public static System.Collections.Generic.List<string> CompareAsObjectTypes { get; }
public static Shouldly.Configuration.ShouldMatchConfigurationBuilder ShouldMatchApprovedDefaults { get; }
public static string? SourceRoot { get; set; }
public static System.IDisposable DisableSourceInErrors() { }
public static bool IsSourceDisabledInErrors() { }
}
Expand Down
1 change: 0 additions & 1 deletion src/Shouldly.Tests/Shouldly.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,4 @@
<PackageReference Include="Microsoft.CSharp" Version="4.7.0" />
<Reference Include="System.Memory" Version="4.5.3" />
</ItemGroup>
<Import Project="..\Shouldly\build.props" />
</Project>
42 changes: 42 additions & 0 deletions src/Shouldly/Internals/SourceCodeTextGetter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Reflection;

namespace Shouldly.Internals;

Expand Down Expand Up @@ -43,6 +45,23 @@ private void ParseStackTrace(StackTrace? stackTrace)
ShouldlyFrameOffset = originatingFrame.index;

var fileName = originatingFrame.frame.GetFileName();
if (fileName?.StartsWith(@"/_/", StringComparison.Ordinal) == true)
{
var sourceRoot = ShouldlyConfiguration.SourceRoot;
if (sourceRoot == null)
{
var assemblyLocation = Assembly.GetExecutingAssembly().Location;
if (assemblyLocation != null)
{
var assemblyDirectory = Path.GetDirectoryName(assemblyLocation);
TryFindGitRepoRoot(assemblyDirectory!, out sourceRoot);
Copy link

Choose a reason for hiding this comment

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

Maybe searching for the SLN could be an alternative to the directory .git. It would remove the dependency to Git if you start a project and add Git afterwards.

    private static bool TryFindProjectFolder(string startDirectory, [NotNullWhen(true)] out string? projectFolder)
    {
        try
        {
            var currentDirectory = new DirectoryInfo(startDirectory);
            while (currentDirectory != null)
            {
                if (IsGitParentDirectory(currentDirectory)
                    || IsSlnParentDirectory(currentDirectory))
                {
                    projectFolder = currentDirectory.FullName;
                    return true;
                }

                currentDirectory = currentDirectory.Parent;
            }
        }
        catch { }

        projectFolder = null;
        return false;
    }

    private static bool IsGitParentDirectory(DirectoryInfo currentDirectory)
    {
        var gitDirectory = Path.Combine(currentDirectory.FullName, ".git");
        return Directory.Exists(gitDirectory);
    }

    private static bool IsSlnParentDirectory(DirectoryInfo currentDirectory)
    {
        var foundFiles = Directory.EnumerateFiles(currentDirectory.FullName, "*.sln");
        return foundFiles.Any();
    }

Further checks could be implemented as needed. I think the best solution would be to look for the CSPROJ of the test projects and parse for the referenced projects to test. But there could be multiple. It would add some complexity.

Copy link
Member Author

@slang25 slang25 Mar 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think we'd get too many false positives, I just checked a handful of open source .NET projects and about half had the .sln file one level in (Newtonsoft.Json, Spectre.Console etc...).

The git approach is quite crude, but might cover 90% of scenarios, for everything else we can tell users to pass a SourceRoot.

For deterministic builds, there an MSBuild ItemGroup item named SourceRoot which ideally we'd want to reuse, but the only way to get it would be some sort of source code generation.

}
}
if (sourceRoot != null)
{
fileName = fileName.Replace("/_/", sourceRoot + Path.PathSeparator);
}
}
_determinedOriginatingFrame = fileName != null && File.Exists(fileName);
_shouldMethod = shouldlyFrame.method.Name;
FileName = fileName;
Expand Down Expand Up @@ -134,4 +153,27 @@ private string GetCodePartFromParameter(int indexOfMethod, string codeLines, str
.RemoveBlock()
.Trim();
}

private static bool TryFindGitRepoRoot(string startDirectory, [NotNullWhen(true)] out string? gitRepoRoot)
{
try
{
var currentDirectory = new DirectoryInfo(startDirectory);
while (currentDirectory != null)
{
var gitDirectory = Path.Combine(currentDirectory.FullName, ".git");
if (Directory.Exists(gitDirectory))
{
gitRepoRoot = currentDirectory.FullName;
return true;
}

currentDirectory = currentDirectory.Parent;
}
}
catch { }

gitRepoRoot = null;
return false;
}
}
4 changes: 2 additions & 2 deletions src/Shouldly/Shouldly.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@
<ItemGroup>
<PackageReference Include="EmptyFiles" Version="4.1.0" PrivateAssets="None" />
<PackageReference Include="DiffEngine" Version="11.0.0" />
<Content Include="build.props" PackagePath="build\Shouldly.props" />
<Content Include="buildMultiTargeting.props" PackagePath="buildMultiTargeting\Shouldly.props" />
<PackageReference Include="Microsoft.CSharp" Version="4.7.0" />
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.1.1" PrivateAssets="All" Condition="$(Configuration) == 'Release'" />
<None Include="..\..\assets\logo_128x128.png" Pack="true" PackagePath="assets" />
</ItemGroup>
<ItemGroup Condition=" '$(TargetFramework)' == 'netstandard2.0' " >
Copy link
Member Author

Choose a reason for hiding this comment

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

This is an unrelated change to this PR, but it was a blocker to using Rider, so have bundled up this fix.

<PackageReference Include="TunnelVisionLabs.ReferenceAssemblyAnnotator" Version="1.0.0-alpha.160" PrivateAssets="all" />
<PackageDownload Include="Microsoft.NETCore.App.Ref" Version="[5.0.0]" />
</ItemGroup>
Expand Down
6 changes: 6 additions & 0 deletions src/Shouldly/ShouldlyConfiguration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,10 @@ public void Dispose()

public static double DefaultFloatingPointTolerance = 0.0d;
public static TimeSpan DefaultTaskTimeout = TimeSpan.FromSeconds(10);

/// <summary>
/// Shouldly can enhance assertion failure messages if it can find the source code at the call site.
/// When using deterministic builds, set this property to explicitly tell Shouldly the root path which symbol paths will be relative to.
Copy link
Member Author

Choose a reason for hiding this comment

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

The wording here is pretty awkward, suggestions welcome!

/// </summary>
public static string? SourceRoot { get; set; } = null;
}
10 changes: 0 additions & 10 deletions src/Shouldly/build.props
View file
Open in desktop

This file was deleted.

4 changes: 0 additions & 4 deletions src/Shouldly/buildMultiTargeting.props
View file
Open in desktop

This file was deleted.