Skip to content
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.

Sign up for GitHub

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

Fix native AoT test scripts #1540

Merged
merged 7 commits into from
Feb 5, 2024
Merged

Fix native AoT test scripts #1540

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ace5667
Support script on Windows
martincostello Jan 20, 2024
34926f4
Validate AoT on macOS
martincostello Jan 20, 2024
a411d77
Fix AoT test script false negatives
martincostello Jan 20, 2024
e6e5bd9
Show publish output
martincostello Jan 23, 2024
853516b
Fix lint warnings
martincostello Jan 23, 2024
05eb22b
Address feedback
martincostello Jan 31, 2024
acc16e4
Verify native AoT on Windows
martincostello Feb 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .github/workflows/verifyaotcompat.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ jobs:
strategy:
fail-fast: false # ensures the entire test matrix is run, even if one permutation fails
matrix:
os: [ ubuntu-latest ]
os: [ ubuntu-latest, windows-latest ]
version: [ net8.0 ]

runs-on: ${{ matrix.os }}
Expand All @@ -19,4 +19,3 @@ jobs:
- name: publish AOT testApp, assert static analysis warning count, and run the app
shell: pwsh
run: .\build\test-aot-compatibility.ps1 ${{ matrix.version }}

17 changes: 13 additions & 4 deletions build/test-aot-compatibility.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ $actualWarningCount = 0

foreach ($line in $($publishOutput -split "`r`n"))
{
if ($line -like "*analysis warning IL*")
if (($line -like "*analysis warning IL*") -or ($line -like "*analysis error IL*"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Under what cases does it emit analysis warning IL vs analysis error IL? Do you need to set "TreatWarningsAsErrors" for this to happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be honest, I'm not sure. I was working locally on a project I knew had AoT issues, but the script wasn't flagging any issues. When I manually ran the commands this script was doing, I found the errors.

Copy link
Contributor

Choose a reason for hiding this comment

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

I can get dotnet publish for PublishAot=true to produce "analysis error IL" only on a 9.0 SDK and if TreatWarningsAsErrors=true. So this seems like the a valid change to make (especially if people are using the script as a starting point).

{
Write-Host $line
$actualWarningCount += 1
Expand All @@ -17,18 +17,27 @@ foreach ($line in $($publishOutput -split "`r`n"))
Write-Host "Actual warning count is:", $actualWarningCount
$expectedWarningCount = 0

pushd $rootDirectory/test/OpenTelemetry.AotCompatibility.TestApp/bin/Release/$targetNetFramework/linux-x64
if ($LastExitCode -ne 0)
{
Write-Host "There was an error while publishing AotCompatibility Test App. LastExitCode is:", $LastExitCode
Write-Host $publishOutput
}

$runtime = $IsWindows ? "win-x64" : ($IsMacOS ? "macos-x64" : "linux-x64")
$app = $IsWindows ? "./OpenTelemetry.AotCompatibility.TestApp.exe" : "./OpenTelemetry.AotCompatibility.TestApp"

Push-Location $rootDirectory/test/OpenTelemetry.AotCompatibility.TestApp/bin/Release/$targetNetFramework/$runtime

Write-Host "Executing test App..."
./OpenTelemetry.AotCompatibility.TestApp
$app
Write-Host "Finished executing test App"

if ($LastExitCode -ne 0)
{
Write-Host "There was an error while executing AotCompatibility Test App. LastExitCode is:", $LastExitCode
}

popd
Pop-Location

$testPassed = 0
if ($actualWarningCount -ne $expectedWarningCount)
Expand Down
4 changes: 2 additions & 2 deletions test/OpenTelemetry.AotCompatibility.TestApp/OpenTelemetry.AotCompatibility.TestApp.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net8.0</TargetFramework>
<PublishAot Condition="!$([MSBuild]::IsOSPlatform('OSX'))">true</PublishAot>
<PublishAot>true</PublishAot>
<TrimmerSingleWarn>false</TrimmerSingleWarn>
<SelfContained>true</SelfContained>
</PropertyGroup>

<ItemGroup>
<!--
When adding projects here please also update the verify-aot-compat job in
.github\workflows\ci.yml so that it runs for the project being added.
.github\workflows\ci.yml so that it runs for the project(s) being added.
-->
<TrimmerRootAssembly Include="OpenTelemetry.Extensions" />
<TrimmerRootAssembly Include="OpenTelemetry.Extensions.Enrichment" />
Expand Down
Loading