Add SDK Symbols tests #19528
Add SDK Symbols tests #19528
Conversation
.../test/Microsoft.DotNet.SourceBuild.SmokeTests/Microsoft.DotNet.SourceBuild.SmokeTests.csproj
Outdated
Show resolved
Hide resolved
| OutputHelper); | ||
|
|
||
| string sdkRoot = Directory.CreateDirectory(Path.Combine(SymbolsTestsRoot, "sdk")).FullName; | ||
| Utilities.ExtractTarball(Config.SdkTarballPath!, sdkRoot, OutputHelper); |
There was a problem hiding this comment.
This already exists at Config.DotNetDirectory once DotNetHelper has been instantiated. Consider using it to avoid extracting this tarball again.
There was a problem hiding this comment.
Fixed with e431f11
| } | ||
| } | ||
| } | ||
|
|
||
| return filesWithoutPDBs; | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
This doesn't seem necessary/intentional.
There was a problem hiding this comment.
Unintentional - will fix.
There was a problem hiding this comment.
Fixed with e431f11
| } | ||
|
|
||
| // Checks if a file has debug data indicating an external companion Pdb. | ||
| public static bool FileHasCompanionPdbInfo(string file, out string guid) |
There was a problem hiding this comment.
Does this need to be public? I don't find any references to it.
There was a problem hiding this comment.
It doesn't need to. I'll update it.
There was a problem hiding this comment.
Fixed with e431f11
| } | ||
|
|
||
| // Checks if a file in Sdk layout requires an external Pdb. | ||
| public static bool FileInSdkLayoutRequiresAPdb(string file, out string guid) |
There was a problem hiding this comment.
{Minor suggestion} Consider putting this in a PdbUtilities method and leave FileUtilities as it was. It avoids the class collision in the tests.
There was a problem hiding this comment.
Fixed with e431f11
| } | ||
|
|
||
| // Checks if a file in Sdk layout requires an external Pdb. | ||
| public static bool FileInSdkLayoutRequiresAPdb(string file, out string guid) |
There was a problem hiding this comment.
The out guid is not intuitive as to what it is for. Please document.
There was a problem hiding this comment.
Fixed with e431f11
Thanks - will fix these. |
…eTests/Microsoft.DotNet.SourceBuild.SmokeTests.csproj Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
Fixes: dotnet/source-build#4330
Fixes: dotnet/source-build#3769
Changes:
CreateSdkSymbolsLayouttask so it can be used by both task and the new testpacks/from missing Pdb detectionNote that dotnet/source-build#3686 was closed in favor of dotnet/source-build#4330