Skip to content

[mono] [tests] Set error for mono_fullaot runtime tests when no suitable tests are found #96902

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

Jump to bottom
Merged
matouskozak merged 2 commits into from
Jan 19, 2024
Merged

[mono] [tests] Set error for mono_fullaot runtime tests when no suitable tests are found #96902

matouskozak merged 2 commits into from
Jan 19, 2024

Conversation

@matouskozak
Copy link
Member

@matouskozak matouskozak commented Jan 12, 2024

Report MSBuild error when compiling selected runtime tests with mono_fullaot and no suitable tests are found.

Motivation

This can happen when locally building selected tests that are disabled in issues.targets. This causes:

  1. Because TestExclusions contain the selected tests, TestScripts won't be created.

    runtime/src/tests/build.proj

    Line 112 in 980491d

    <TestScripts Include="@(AllTestScripts)" Exclude="@(TestExclusions)" />
  2. This subsequently causes TestDirs to no exists, leading to enumarating all dlls on the disk (/**/*.dll) in

    runtime/src/tests/build.proj

    Line 127 in 980491d

    <TestsAndAssociatedAssemblies Include="%(TestDirs.Identity)/**/*.dll" Exclude="@(NoMonoAotAssemblyPaths)" />

Previous behavior

Build doesn't get stopped and the following warning is reported:

MSBUILD : warning MSB5029: The value "/**/*.dll" of the "Include" attribute in element <ItemGroup> in file "<RUNTIME_ROOT>runtime/src/tests/build.proj (122,37)" is a wildcard that results in enumerating all files on the drive, which was likely not intended. Check that referenced properties are always defined.

New behavior

Build gets canceled and error is reported: error : No tests found for Mono AOT compilation.

@ghost ghost assigned matouskozak Jan 12, 2024
@ghost ghost added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jan 12, 2024
@matouskozak matouskozak added area-Infrastructure-mono and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jan 12, 2024
@ghost
Copy link

ghost commented Jan 12, 2024

Tagging subscribers to this area: @directhex
See info in area-owners.md if you want to be subscribed.

Issue Details

Report MSBuild error when compiling selected runtime tests with mono_fullaot and no suitable tests are found.

Motivation

This can happen when locally building selected tests that are disabled in issues.targets. This causes:

  1. Because TestExclusions contain the selected tests, TestScripts won't be created.

    runtime/src/tests/build.proj

    Line 112 in 980491d

    <TestScripts Include="@(AllTestScripts)" Exclude="@(TestExclusions)" />
  2. This subsequently causes TestDirs to no exists, leading to enumarating all dlls on the disk (/**/*.dll) in

    runtime/src/tests/build.proj

    Line 127 in 980491d

    <TestsAndAssociatedAssemblies Include="%(TestDirs.Identity)/**/*.dll" Exclude="@(NoMonoAotAssemblyPaths)" />

Previous behavior

Build doesn't get stopped and the following warning is reported:

MSBUILD : warning MSB5029: The value "/**/*.dll" of the "Include" attribute in element <ItemGroup> in file "<RUNTIME_ROOT>runtime/src/tests/build.proj (122,37)" is a wildcard that results in enumerating all files on the drive, which was likely not intended. Check that referenced properties are always defined.

New behavior

Build gets canceled and error is reported: error : No tests found for Mono AOT compilation.

Author: matouskozak
Assignees: matouskozak
Labels:

area-Infrastructure-mono, needs-area-label
Milestone: -

@build-analysis build-analysis bot mentioned this pull request Jan 12, 2024
Closed
@matouskozak matouskozak marked this pull request as ready for review January 15, 2024 10:04
ivanpovazan
ivanpovazan approved these changes Jan 15, 2024
View reviewed changes
Copy link
Member

@ivanpovazan ivanpovazan left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM!

Only two nits:

  1. Would it make sense to check on TestAssemblies instead, as there is already a condition filtering out paths that exist on line 115?
  2. Maybe the error message can include additional information about tests exclusions, something like: ... Make sure that desired test cases are not excluded from AOT compilation

@matouskozak matouskozak merged commit f6824bc into dotnet:main Jan 19, 2024
tmds pushed a commit to tmds/runtime that referenced this pull request Jan 23, 2024
@matouskozak @tmds
[mono] [tests] Set error for mono_fullaot runtime tests when no suita…
ff09a42 
…ble tests are found (dotnet#96902)
@matouskozak matouskozak mentioned this pull request Feb 16, 2024
Closed
2 tasks
@github-actions github-actions bot locked and limited conversation to collaborators Feb 19, 2024
@matouskozak matouskozak deleted the feature/tests/error-no-tests-fullaot branch October 3, 2024 13:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@akoeplinger akoeplinger akoeplinger approved these changes

@ivanpovazan ivanpovazan ivanpovazan approved these changes

@directhex directhex Awaiting requested review from directhex

@trylek trylek Awaiting requested review from trylek

Assignees

@matouskozak matouskozak

Labels

area-Infrastructure-mono

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@matouskozak @akoeplinger @ivanpovazan