Skip to content

Conversation

@slang25
Copy link
Member

@slang25 slang25 commented Apr 22, 2023

Following on from #883, it turns out that as suspected there is a need to support building and running tests in separate steps while also using deterministic builds.

This PR adds:

  • Refactoring deterministic build code to DeterministicBuildHelpers
  • A method to check for paths which "look deterministic" if the build tooling fails for some reason. These paths are prefixed with /_/, /_1/ etc.. This is used in the ShouldMatchApproved method, where an accurate resolved location is essential.
  • New MSBuild targets which produce a file during build capturing the path maps and copying this to the output folder, then reading this value during VSTest and setting it as an environment variable.
  • In the case where dotnet test is ran on it's own, a build will be performed but it will be "within" the VSTest target, so wont run before it. In this case we set the environment variable directly, as we know the tests will run later and in the same process tree.
  • A DisableShouldlyPathMaps build property users can set to true to completely disable this behaviour, just in case.

@slang25
Copy link
Member Author

slang25 commented Apr 22, 2023

@sungam3r this should solve the errors you reported, I'll work on testing this on one of the repos you linked to

@slang25
Copy link
Member Author

slang25 commented Apr 23, 2023

I've recreated your issue on AspNetCore.Diagnostics.HealthChecks @sungam3r, and have confirmed that this new package fixes the issue.

I'll leave this PR open for comments for a day, if there are no comments then I'll look to get a 4.2.1 release out 🙂

@slang25 slang25 marked this pull request as ready for review April 23, 2023 10:38
@sungam3r
Copy link
Contributor

Thanks.

@slang25 slang25 merged commit c26fcfc into master Apr 24, 2023
@slang25 slang25 deleted the better-determistic-builds branch April 24, 2023 09:02
@sungam3r
Copy link
Contributor

4.2.1 seems work

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.

3 participants