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.

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

[Tests] Remove Builder.UseDotNet #8441

Merged
merged 1 commit into from
Nov 8, 2023
Merged

Conversation

pjcollins
Copy link
Member

@pjcollins pjcollins commented Oct 19, 2023

The .NET versus Classic switches have been removed from the MSBuild
tests. Tests / asserts that only applied to classic have been removed.
Test parameters that only applied to classic (aapt vs aapt2) have
been removed. Some project properties that only applied to classic
(UseLatestPlatformSdk) have been removed.

@pjcollins pjcollins force-pushed the rmusedotnet branch 4 times, most recently from 91f3461 to d735498 Compare October 23, 2023 15:54
@pjcollins pjcollins added the do-not-merge PR should not be merged. label Oct 23, 2023
@pjcollins pjcollins marked this pull request as ready for review October 23, 2023 20:11
@pjcollins
Copy link
Member Author

This should be ready for review. Since there is a lot changing/being removed here we may want to hold off on merging for a little while however, if only to make any upcoming backports needed for .NET 8 easier.

Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

I got partway through...

image

As for concerns about conflicts, I would be ok just taking this back to .NET 8 servicing. It's test-only changes, and shouldn't break anything.

@pjcollins pjcollins force-pushed the rmusedotnet branch 2 times, most recently from a9e1720 to 7c9f315 Compare October 25, 2023 16:26
@pjcollins pjcollins requested a review from grendello October 25, 2023 22:16
@pjcollins
Copy link
Member Author

There's one issue that's been happening here consistently that I can't seem to track down, and couldn't repro locally:

 Failed RunWithLLVMEnabled [1 m 41 s]
  Error Message:
     LLVM .so files not loaded:
10-25 19:11:59.456  7936  7936 I monodroid-assembly: Failed to load shared library '/data/app/com.xamarin.runwithllvmenabled-yiFod3LQK46wSU9vJ8UUfw==/split_config.x86_64.apk!/lib/x86_64/libmono-profiler-aot.so'. dlopen failed: library "/data/app/com.xamarin.runwithllvmenabled-yiFod3LQK46wSU9vJ8UUfw==/split_config.x86_64.apk!/lib/x86_64/libmono-profiler-aot.so" not found
10-25 19:11:59.456  7936  7936 I monodroid-assembly: Failed to load shared library '/data/app/com.xamarin.runwithllvmenabled-yiFod3LQK46wSU9vJ8UUfw==/lib/x86_64/libmono-profiler-aot.so'. dlopen failed: library "/data/app/com.xamarin.runwithllvmenabled-yiFod3LQK46wSU9vJ8UUfw==/lib/x86_64/libmono-profiler-aot.so" not found
  Expected: 0
  But was:  2

I don't think a version of libmono-profiler-aot.so exists for .NET Android? @grendello do you have any thoughts as to why we're trying to load this?

@jonathanpeppers
Copy link
Member

libmono-profiler-aot.so would only exist in .NET 6+, if you added my NuGet package:

https://www.nuget.org/packages/Mono.AotProfiler.Android

@grendello
Copy link
Contributor

@pjcollins we'd load it if the debug.mono.profile sysprop were set to contain something like e.g. aot:output=file/path

@pjcollins
Copy link
Member Author

@pjcollins we'd load it if the debug.mono.profile sysprop were set to contain something like e.g. aot:output=file/path

Ooh great thanks, I think this explains it. It looks like this test is running during the same job as BuildBasicApplicationAndAotProfileIt now, so I'll update that fixture to make sure we clear the profiling prop.

@pjcollins pjcollins force-pushed the rmusedotnet branch 3 times, most recently from 3bf7df2 to 1eb0f83 Compare October 27, 2023 21:33
@pjcollins pjcollins removed the do-not-merge PR should not be merged. label Oct 29, 2023
@pjcollins
Copy link
Member Author

I think this should be good to go whenever there's some time to review.

@pjcollins
Copy link
Member Author

Not yet sure what's up with the latest resource designer test failures but they seem to be happening on other PRs, and I haven't been able to repro them locally.

The .NET versus Classic switches have been removed from the MSBuild
tests.  Tests / asserts that only applied to classic have been removed.
Test parameters that only applied to classic (`aapt` vs `aapt2`) have
been removed.  Some project properties that only applied to classic
(`UseLatestPlatformSdk`) have been removed.
@dellis1972
Copy link
Contributor

@pjcollins the designer tests have been fixed, if you rebase they should go away 👍

@pjcollins pjcollins merged commit c13c94d into dotnet:main Nov 8, 2023
47 checks passed
@pjcollins pjcollins deleted the rmusedotnet branch November 8, 2023 17:35
grendello added a commit that referenced this pull request Nov 9, 2023
* main:
  [tests] Remove Builder.UseDotNet (#8441)
@github-actions github-actions bot locked and limited conversation to collaborators Jan 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants