Skip to content

Conversation

@mthalman
Copy link
Member

Now that dotnet/source-build-externals#179 has been integrated, the SB SDK diff tests are failing because the assembly versions are now correct. This causes there to be no diff between the MSFT and SB SDKs for assembly versions.

The test infra has been updated to allow for a call to git diff to return a 0 exit code, meaning no diffs. And the baseline file is also updated to no longer contain the IdentityModel assemblies as diffs.

The SDK diff test pipeline won't yet pass completely with this change. It's still failing in the Ubuntu legs because the VMR doesn't yet have passing build jobs for that distro. That won't happen until dotnet/source-build-externals#183 is integrated.

@mthalman mthalman requested a review from a team as a code owner August 10, 2023 13:59
(Process Process, string StdOut, string StdErr) diffResult =
ExecuteHelper.ExecuteProcess("git", $"diff --no-index {file1Path} {file2Path}", outputHelper);
Assert.Equal(1, diffResult.Process.ExitCode);
Assert.True(diffResult.Process.ExitCode == 0 || diffResult.Process.ExitCode == 1);
Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense to retain this Assert anymore? It looks like the exit code from git diff is either 0 or 1.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah probably not. It looks like other error conditions return 1 as well so this doesn't add any value.

@mthalman mthalman requested a review from MichaelSimons August 10, 2023 14:14
@mthalman mthalman enabled auto-merge (squash) August 10, 2023 14:33
@mthalman mthalman merged commit 2029cca into dotnet:main Aug 10, 2023
@mthalman mthalman deleted the dev/mthalman/diff-test branch August 10, 2023 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants