-
Notifications
You must be signed in to change notification settings - Fork 330
Enable InternalsVisibleTo for UnitTests in Package mode #4300
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.
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
5ca720a
422bcf6
fd1a2ff
1499698
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -79,51 +79,62 @@ steps: | |
| condition: succeededOrFailed() | ||
|
|
||
| - ${{if eq(parameters.operatingSystem, 'Windows')}}: | ||
| - ${{if eq(parameters.referenceType, 'Project')}}: | ||
| - task: DotNetCoreCLI@2 | ||
| displayName: 'Run Unit Tests ${{parameters.msbuildArchitecture }}' | ||
| condition: and(eq(variables['setupSucceeded'], 'true'), succeededOrFailed()) | ||
| inputs: | ||
| command: build | ||
| projects: build.proj | ||
| ${{ if eq(parameters.msbuildArchitecture, 'x64') }}: | ||
| arguments: >- | ||
| -t:TestSqlClientUnit | ||
| -p:TestFramework=${{ parameters.targetFramework }} | ||
| -p:Configuration=${{ parameters.buildConfiguration }} | ||
| -p:TestResultsFolderPath=TestResults | ||
| ${{ else }}: # x86 | ||
| arguments: >- | ||
| -t:TestSqlClientUnit | ||
| -p:TestFramework=${{ parameters.targetFramework }} | ||
| -p:Configuration=${{ parameters.buildConfiguration }} | ||
| -p:DotnetPath=${{ parameters.dotnetx86RootPath }} | ||
| -p:TestResultsFolderPath=TestResults | ||
| - task: DotNetCoreCLI@2 | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now we will see unit test runs in the PR/CI Package mode pipelines. Neither of those perform signing, so InternalsVisibleTo will be granted by the SqlClient project. There is no new risk here. Apps can already access everything via reflection. If we ever publish unsigned assemblies, we have much bigger problems than IVT. |
||
| displayName: 'Run Unit Tests ${{parameters.msbuildArchitecture }}' | ||
| condition: and(eq(variables['setupSucceeded'], 'true'), succeededOrFailed()) | ||
| inputs: | ||
| command: build | ||
| projects: build.proj | ||
| ${{ if eq(parameters.msbuildArchitecture, 'x64') }}: | ||
| arguments: >- | ||
| -t:TestSqlClientUnit | ||
| -p:TestFramework=${{ parameters.targetFramework }} | ||
| -p:ReferenceType=${{ parameters.referenceType }} | ||
| -p:Configuration=${{ parameters.buildConfiguration }} | ||
| -p:PackageVersionSqlClient=${{ parameters.mdsPackageVersion }} | ||
| -p:PackageVersionSqlServer=${{ parameters.sqlServerPackageVersion }} | ||
| -p:TestResultsFolderPath=TestResults | ||
| ${{ else }}: # x86 | ||
| arguments: >- | ||
| -t:TestSqlClientUnit | ||
| -p:TestFramework=${{ parameters.targetFramework }} | ||
| -p:ReferenceType=${{ parameters.referenceType }} | ||
| -p:Configuration=${{ parameters.buildConfiguration }} | ||
| -p:PackageVersionSqlClient=${{ parameters.mdsPackageVersion }} | ||
| -p:PackageVersionSqlServer=${{ parameters.sqlServerPackageVersion }} | ||
| -p:DotnetPath=${{ parameters.dotnetx86RootPath }} | ||
| -p:TestResultsFolderPath=TestResults | ||
|
|
||
| - task: DotNetCoreCLI@2 | ||
| displayName: 'Run Flaky Unit Tests ${{parameters.msbuildArchitecture }}' | ||
| condition: and(eq(variables['setupSucceeded'], 'true'), succeededOrFailed()) | ||
| inputs: | ||
| command: build | ||
| projects: build.proj | ||
| ${{ if eq(parameters.msbuildArchitecture, 'x64') }}: | ||
| arguments: >- | ||
| -t:TestSqlClientUnit | ||
| -p:TestFramework=${{ parameters.targetFramework }} | ||
| -p:Configuration=${{ parameters.buildConfiguration }} | ||
| -p:TestFilters="category=flaky" | ||
| -p:TestResultsFolderPath=TestResults | ||
| -p:TestCodeCoverage=false | ||
| ${{ else }}: # x86 | ||
| arguments: >- | ||
| -t:TestSqlClientUnit | ||
| -p:TestFramework=${{ parameters.targetFramework }} | ||
| -p:Configuration=${{ parameters.buildConfiguration }} | ||
| -p:DotnetPath=${{ parameters.dotnetx86RootPath }} | ||
| -p:TestFilters="category=flaky" | ||
| -p:TestResultsFolderPath=TestResults | ||
| -p:TestCodeCoverage=false | ||
| continueOnError: true | ||
| - task: DotNetCoreCLI@2 | ||
| displayName: 'Run Flaky Unit Tests ${{parameters.msbuildArchitecture }}' | ||
| condition: and(eq(variables['setupSucceeded'], 'true'), succeededOrFailed()) | ||
| inputs: | ||
| command: build | ||
| projects: build.proj | ||
| ${{ if eq(parameters.msbuildArchitecture, 'x64') }}: | ||
| arguments: >- | ||
| -t:TestSqlClientUnit | ||
| -p:TestFramework=${{ parameters.targetFramework }} | ||
| -p:ReferenceType=${{ parameters.referenceType }} | ||
| -p:Configuration=${{ parameters.buildConfiguration }} | ||
| -p:PackageVersionSqlClient=${{ parameters.mdsPackageVersion }} | ||
| -p:PackageVersionSqlServer=${{ parameters.sqlServerPackageVersion }} | ||
| -p:TestFilters="category=flaky" | ||
| -p:TestResultsFolderPath=TestResults | ||
| -p:TestCodeCoverage=false | ||
| ${{ else }}: # x86 | ||
| arguments: >- | ||
| -t:TestSqlClientUnit | ||
| -p:TestFramework=${{ parameters.targetFramework }} | ||
| -p:ReferenceType=${{ parameters.referenceType }} | ||
| -p:Configuration=${{ parameters.buildConfiguration }} | ||
| -p:PackageVersionSqlClient=${{ parameters.mdsPackageVersion }} | ||
| -p:PackageVersionSqlServer=${{ parameters.sqlServerPackageVersion }} | ||
| -p:DotnetPath=${{ parameters.dotnetx86RootPath }} | ||
| -p:TestFilters="category=flaky" | ||
| -p:TestResultsFolderPath=TestResults | ||
| -p:TestCodeCoverage=false | ||
| continueOnError: true | ||
|
|
||
| - task: DotNetCoreCLI@2 | ||
| displayName: 'Run Functional Tests ${{parameters.msbuildArchitecture }}' | ||
|
|
@@ -261,33 +272,38 @@ steps: | |
| continueOnError: true | ||
|
|
||
| - ${{ else }}: # Linux or macOS | ||
| - ${{if eq(parameters.referenceType, 'Project')}}: | ||
| - task: DotNetCoreCLI@2 | ||
| displayName: 'Run Unit Tests' | ||
| condition: and(eq(variables['setupSucceeded'], 'true'), succeededOrFailed()) | ||
| inputs: | ||
| command: build | ||
| projects: build.proj | ||
| arguments: >- | ||
| -t:TestSqlClientUnit | ||
| -p:TestFramework=${{ parameters.targetFramework }} | ||
| -p:Configuration=${{ parameters.buildConfiguration }} | ||
| -p:TestResultsFolderPath=TestResults | ||
| - task: DotNetCoreCLI@2 | ||
| displayName: 'Run Unit Tests' | ||
| condition: and(eq(variables['setupSucceeded'], 'true'), succeededOrFailed()) | ||
| inputs: | ||
| command: build | ||
| projects: build.proj | ||
| arguments: >- | ||
| -t:TestSqlClientUnit | ||
| -p:TestFramework=${{ parameters.targetFramework }} | ||
| -p:ReferenceType=${{ parameters.referenceType }} | ||
| -p:Configuration=${{ parameters.buildConfiguration }} | ||
| -p:PackageVersionSqlClient=${{ parameters.mdsPackageVersion }} | ||
| -p:PackageVersionSqlServer=${{ parameters.sqlServerPackageVersion }} | ||
| -p:TestResultsFolderPath=TestResults | ||
|
|
||
| - task: DotNetCoreCLI@2 | ||
| displayName: 'Run Flaky Unit Tests' | ||
| condition: and(eq(variables['setupSucceeded'], 'true'), succeededOrFailed()) | ||
| inputs: | ||
| command: build | ||
| projects: build.proj | ||
| arguments: >- | ||
| -t:TestSqlClientUnit | ||
| -p:TestFramework=${{ parameters.targetFramework }} | ||
| -p:Configuration=${{ parameters.buildConfiguration }} | ||
| -p:TestFilters="category=flaky" | ||
| -p:TestResultsFolderPath=TestResults | ||
| -p:TestCodeCoverage=false | ||
| continueOnError: true | ||
| - task: DotNetCoreCLI@2 | ||
| displayName: 'Run Flaky Unit Tests' | ||
| condition: and(eq(variables['setupSucceeded'], 'true'), succeededOrFailed()) | ||
| inputs: | ||
| command: build | ||
| projects: build.proj | ||
| arguments: >- | ||
| -t:TestSqlClientUnit | ||
| -p:TestFramework=${{ parameters.targetFramework }} | ||
| -p:ReferenceType=${{ parameters.referenceType }} | ||
| -p:Configuration=${{ parameters.buildConfiguration }} | ||
| -p:PackageVersionSqlClient=${{ parameters.mdsPackageVersion }} | ||
| -p:PackageVersionSqlServer=${{ parameters.sqlServerPackageVersion }} | ||
| -p:TestFilters="category=flaky" | ||
| -p:TestResultsFolderPath=TestResults | ||
| -p:TestCodeCoverage=false | ||
| continueOnError: true | ||
|
|
||
| - task: DotNetCoreCLI@2 | ||
| displayName: 'Run Functional Tests' | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,13 +62,26 @@ | |
|
|
||
| <!-- Strong name signing ============================================= --> | ||
| <!-- Strong naming is being done in Directory.Build.props --> | ||
| <!-- If we're not signing, we are permitted to expose our internals to tests. --> | ||
|
|
||
| <!-- Unsigned: expose internals to UnitTests in any reference mode. --> | ||
| <ItemGroup Condition="'$(SigningKeyPath)' == ''"> | ||
| <AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleToAttribute"> | ||
| <_Parameter1>UnitTests</_Parameter1> | ||
| </AssemblyAttribute> | ||
| </ItemGroup> | ||
|
|
||
| <!-- | ||
| Signed + Package mode: expose internals to UnitTests signed with the test key. | ||
| Signed + Project mode is intentionally omitted: production-signed assemblies should not | ||
| grant internal access to test assemblies during local development. Tests that need internals | ||
| in Project mode run unsigned; only the CI package-validation pipeline uses signed packages. | ||
| --> | ||
| <ItemGroup Condition="'$(SigningKeyPath)' != '' AND '$(ReferenceType)' == 'Package'"> | ||
| <AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleToAttribute"> | ||
| <_Parameter1>UnitTests, PublicKey=00240000048000001401000006020000002400005253413100080000010001003D19684676DA365F331D00CE7BD4B8EF03E74102F39A5681B40622703D68F0298ECACECC723D3FFC1EA9365AF4958578550EA1EBEEC084B0B3757F3762449F5365E872802A4B548056760764FAD062BFEE81ED26183109AD46810E7E6E965419D0A10473680144D20C1BFE1027A5F586CA987523C06F5C126C44EA7D4F51EB023867A9F294315F95775ACEFD2D678186919458DFCCB4DE2E9F53AEFC766C7CBCEC474ED21C1616E5A9414D366D91D121C39F5FE6641295ADC058EF3FB10593BCDE2E82D9F217C2634909EEF496CD53AE78ABBEA572B871D72EBFC5378205950ABA97C7CCC2B9635D96933D5F9C9624D71FF53EE2094CF3A6BD38534D66E414B7</_Parameter1> | ||
|
paulmedynski marked this conversation as resolved.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I created a new signing key specifically for tests. It lives in the ADO.Net project as a secure file. This is its public key. |
||
| </AssemblyAttribute> | ||
| </ItemGroup> | ||
|
|
||
| <!-- Build Output ==================================================== --> | ||
| <PropertyGroup> | ||
| <ArtifactPath>$(RepoRoot)artifacts/</ArtifactPath> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,13 @@ | |
| <IsTestProject>false</IsTestProject> | ||
| </PropertyGroup> | ||
|
|
||
| <!-- Strong name signing ============================================= --> | ||
| <!-- Sign with the test key so signed test assemblies can reference this project. --> | ||
| <PropertyGroup Condition="'$(TestSigningKeyPath)' != ''"> | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any projects used by a signed test project must also be signed. |
||
| <SignAssembly>true</SignAssembly> | ||
| <AssemblyOriginatorKeyFile>$(TestSigningKeyPath)</AssemblyOriginatorKeyFile> | ||
| </PropertyGroup> | ||
|
|
||
| <!-- Target Frameworks =============================================== --> | ||
| <PropertyGroup> | ||
| <TargetFrameworks>net8.0;net9.0;net10.0</TargetFrameworks> | ||
|
|
@@ -26,7 +33,10 @@ | |
| <!-- References ====================================================== --> | ||
| <!-- Test target reference --> | ||
| <ItemGroup> | ||
| <ProjectReference Include="$(RepoRoot)src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj" /> | ||
| <ProjectReference Include="$(RepoRoot)src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj" | ||
| Condition="'$(ReferenceType)' != 'Package'" /> | ||
| <PackageReference Include="Microsoft.Data.SqlClient" | ||
| Condition="'$(ReferenceType)' == 'Package'" /> | ||
| </ItemGroup> | ||
|
|
||
| <!-- References for netfx --> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,22 +36,51 @@ | |
| </None> | ||
| </ItemGroup> | ||
|
|
||
| <!-- Strong name signing ============================================= --> | ||
| <!-- When a test signing key is provided, sign UnitTests so IVT from signed SqlClient works. --> | ||
| <PropertyGroup Condition="'$(TestSigningKeyPath)' != ''"> | ||
| <SignAssembly>true</SignAssembly> | ||
| <AssemblyOriginatorKeyFile>$(TestSigningKeyPath)</AssemblyOriginatorKeyFile> | ||
| </PropertyGroup> | ||
|
|
||
| <!-- References ====================================================== --> | ||
| <!-- Test target reference --> | ||
| <ItemGroup> | ||
| <!-- | ||
| The Unit Test project only supports ReferenceType = Project since it needs access to the | ||
| internals of sibling packages. | ||
| --> | ||
| <ItemGroup Condition="'$(ReferenceType)' != 'Package'"> | ||
| <ProjectReference Include="$(RepoRoot)src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj" /> | ||
| <ProjectReference Include="$(RepoRoot)src/Microsoft.SqlServer.Server/Microsoft.SqlServer.Server.csproj" /> | ||
| </ItemGroup> | ||
|
|
||
| <Target Name="ValidateReferenceType" | ||
| BeforeTargets="Restore;PrepareForBuild" | ||
| Condition="'$(ReferenceType)' != 'Project'"> | ||
| <Error Text="Microsoft.Data.SqlClient.UnitTests.csproj only supports ReferenceType=Project." /> | ||
| </Target> | ||
| <!-- | ||
| Package mode: Unit tests exercise internal types that only exist in the implementation assembly | ||
| (runtimes/{rid}/lib/{tfm}/), not in the ref assembly (ref/{tfm}/). We exclude the ref | ||
| compile asset and reference the runtime DLL directly so the compiler can see internals. | ||
|
|
||
| TODO: This is fragile, depending on our SqlClient NuGet package layout and supported TFMs. This | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that if/when our other test projects (i.e. Azure.Test) adopt this Package+Signing approach, those projects won't need this foo since they don't produce and package ref assemblies. |
||
| could all be avoided if we didn't ship ref assemblies. | ||
| --> | ||
| <PropertyGroup Condition="'$(ReferenceType)' == 'Package'"> | ||
| <_SqlClientRid Condition="'$(OS)' != 'Windows_NT'">unix</_SqlClientRid> | ||
| <_SqlClientRid Condition="'$(OS)' == 'Windows_NT'">win</_SqlClientRid> | ||
| <!-- Map test TFM to the closest available package TFM (package ships net8.0/net9.0/net462) --> | ||
| <_SqlClientPackageTfm Condition="'$(TargetFramework)' == 'net8.0'">net8.0</_SqlClientPackageTfm> | ||
| <_SqlClientPackageTfm Condition="'$(TargetFramework)' == 'net462'">net462</_SqlClientPackageTfm> | ||
| <_SqlClientPackageTfm Condition="'$(_SqlClientPackageTfm)' == ''">net9.0</_SqlClientPackageTfm> | ||
| </PropertyGroup> | ||
| <ItemGroup Condition="'$(ReferenceType)' == 'Package'"> | ||
| <!-- Brings in transitive dependencies and runtime deployment; ExcludeAssets="compile" prevents | ||
| the compiler from using the ref assembly (which only contains public API surface). | ||
| GeneratePathProperty="true" causes NuGet restore to emit $(PkgMicrosoft_Data_SqlClient) | ||
| pointing to the extracted package folder in the global packages cache. --> | ||
| <PackageReference Include="Microsoft.Data.SqlClient" | ||
| GeneratePathProperty="true" | ||
| ExcludeAssets="compile" /> | ||
| <!-- Compile against the implementation assembly so the compiler can resolve internal types | ||
| exposed via InternalsVisibleTo. The path uses $(PkgMicrosoft_Data_SqlClient) generated | ||
| above to locate the runtime DLL inside the NuGet package layout. --> | ||
| <Reference Include="Microsoft.Data.SqlClient" | ||
| HintPath="$(PkgMicrosoft_Data_SqlClient)/runtimes/$(_SqlClientRid)/lib/$(_SqlClientPackageTfm)/Microsoft.Data.SqlClient.dll" /> | ||
| <PackageReference Include="Microsoft.SqlServer.Server" /> | ||
| </ItemGroup> | ||
|
|
||
| <!-- References for netframework --> | ||
| <ItemGroup Condition="'$(TargetFramework)' == 'net462'"> | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.