Docker tests NetFramework-48 and Licensing-Unattended pass without PostSharp running (-t:Restore -t:Build) #173

Summary

Two Docker tests build their project with -t:Restore -t:Build and check only the exit code of MSBuild. On a single-target project, that command builds without PostSharp, so the tests pass without weaving anything:

• Tests/Core/DockerTests/NetFramework-48 (legacy project NetFrameworkProject, msbuild -m -t:Restore -t:Build)
• Tests/Core/DockerTests/Licensing-Unattended (SDK-style project LicensingUnattendedProject, single TargetFramework, dotnet msbuild -t:Restore -t:Build)

The Licensing-Unattended defect matters most. That test passes MaxFreeEnhancedLocInProject=1 so that a successful build proves that the unattended licence was granted. When PostSharp does not run, the build succeeds anyway, and the test proves nothing.

Found on the branch of postsharp/PostSharp#134 (topic/2027.0/26-09-25-package-build-dotnet-pack). The same pattern caused the pipe server test to pass vacuously; commit 8a867cfb7b on topic/2027.0/124-shutdown-command fixed that test and added the rule to Tests/Core/DockerTests/CLAUDE.md.

Measurements

The measurements were taken on the host, not in the containers. The environment was MSBuild from Visual Studio 18 Insiders and .NET SDK 11.0.100-rc.1. The projects and Directory.Build.props were taken from the PR branch, and each build started from a clean tree with no .artifacts directory. A detailed file log (-flp:...;Verbosity=detailed) was searched for targets whose name contains PostSharp, and the output program was run.

The runs used the local package PostSharp 2027.0.0.1033-local. A first run of -t:Restore -t:Build on NetFrameworkProject used the local package 2027.0.0.1013. The task of that package cannot load (SharpCrafters.Backstage.Threading is missing), and -restore fails with MSB4018 in PostSharpExtractTools. Yet the -t:Restore -t:Build build with that package succeeded. The targets of the package were therefore never imported.

Common.ps1 passes the command to the container unchanged and runs no separate restore. On a build agent the .artifacts directory does not exist, so the measured case is the one that CI runs.

A second observation, from the same runs: NetFrameworkProject sets OutputPath per configuration, so /p:BaseOutputPath=../.artifacts/NetFramework-48/bin/ does not move the output. The output was written to NetFrameworkProject/bin/Release/, inside the shared project directory. /p:OutputPath=../.artifacts/NetFramework-48/bin/ put it under .artifacts.

Cause

The following is inferred from known MSBuild behaviour; it was not traced in MSBuild.

-t:Restore -t:Build evaluates the project once, before the restore writes obj/<project>.nuget.g.props and .nuget.g.targets. These files import build/PostSharp.props and build/PostSharp.targets from the package. The Build target therefore runs on an evaluation without PostSharp. -restore runs the restore in a separate build, and then evaluates the project again.

NetCoreProject is not affected because it multi-targets. The outer build starts one inner build per TargetFramework with the MSBuild task. Each inner build is a new evaluation that happens after the restore. That explanation is also an inference.

A second run over an existing .artifacts directory would find the nuget.g.* files at evaluation time and weave. So a test can pass for the right reason locally and vacuously on CI. This was not measured.

Proposed change

1. In NetFramework-48/RunTest.ps1 and Licensing-Unattended/RunTest.ps1, replace -t:Restore -t:Build with -restore -t:Build.
2. In NetFramework-48/RunTest.ps1, replace /p:BaseOutputPath=... with /p:OutputPath=../.artifacts/NetFramework-48/bin/.
3. Make both tests assert that PostSharp ran. For example, run the built program and require the line of TestAspect (Hello from PostSharp!!! / Hello from PostSharp!). A check of the exit code alone cannot detect this failure.
4. Optionally, apply the same change and assertion to the NetCore-* tests. They weave today only because NetCoreProject multi-targets, and a change to a single target would make them vacuous without warning.

-- Claude for Gael Fraiteur

Moved from the private issue tracker (postsharp/PostSharp#139).