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

[Android] Add option to skip generating apk for a specific project #80909

Merged

Conversation

simonrozsival
Copy link
Member

Follow-up to #80391, supersedes #80874

/cc @lambdageek

The Android.Device_Emulator.StartupHook.Test.csproj functional test references StartupHookForFunctionalTest.csproj. Counterintuitively, this causes the StartupHookForFunctionalTest project to be built into an .apk bundle and this APK is then executed as if it were a test executable and this "test" then obviously fails.

https://github.com/dotnet/runtime/blob/main/src/tests/FunctionalTests/Android/Device_Emulator/StartupHook/Android.Device_Emulator.StartupHook.Test.csproj#L16-L18

I tried several ways of preventing the referenced project to be built into an .apk and the simplest and cleanest solution I arrived at is a new property that can be defined in the .csproj.

@ghost
Copy link

ghost commented Jan 20, 2023

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

Issue Details

Follow-up to #80391, supersedes #80874

/cc @lambdageek

The Android.Device_Emulator.StartupHook.Test.csproj functional test references StartupHookForFunctionalTest.csproj. Counterintuitively, this causes the StartupHookForFunctionalTest project to be built into an .apk bundle and this APK is then executed as if it were a test executable and this "test" then obviously fails.

https://github.com/dotnet/runtime/blob/main/src/tests/FunctionalTests/Android/Device_Emulator/StartupHook/Android.Device_Emulator.StartupHook.Test.csproj#L16-L18

I tried several ways of preventing the referenced project to be built into an .apk and the simplest and cleanest solution I arrived at is a new property that can be defined in the .csproj.

Author: simonrozsival
Assignees: simonrozsival
Labels:

area-Infrastructure-mono

Milestone: -

@simonrozsival simonrozsival force-pushed the android-skip-generating-app-bundle branch from 4df305f to 289e726 Compare January 20, 2023 11:37
@dotnet dotnet deleted a comment from azure-pipelines bot Jan 20, 2023
@simonrozsival
Copy link
Member Author

/azp run runtime-android

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@simonrozsival
Copy link
Member Author

The failing test on Android is #80863

<Target Name="_AndroidGenerateAppBundle" DependsOnTargets="_AndroidGenerateRuntimeConfig">
<Target
Name="_AndroidGenerateAppBundle"
Condition="$(_SkipAndroidGenerateAppBundle) != 'true'"
Copy link
Member

Choose a reason for hiding this comment

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

The WasmApp.targets makes a property IsBrowserWasmProject that is only true if the OutputType is not Library, by default. and then gates all the wasm-specific build logic based on that.

<IsBrowserWasmProject Condition="'$(IsBrowserWasmProject)' == '' and '$(OutputType)' != 'Library'">true</IsBrowserWasmProject>

Maybe something like that should be done for Android too? Although I didn't look if the test runners are Exe or Library and if all the functional tests are exe's, too.

/cc @radical

Copy link
Member Author

Choose a reason for hiding this comment

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

I looked into the option of using OutputType but from what I understand we can't use it because at least the library tests don't set the OutputType and afaik the default is Library.

Copy link
Member

Choose a reason for hiding this comment

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

How does it work on wasm then?

It shouldn't matter if the library tests are OutputType=Library - the test runner is an exe, right?

But I'm not sure what step is responsible for taking a FOO.Tests.dll and the apropriate test runner and putting them together into a single app.

Copy link
Member

@steveisok steveisok Jan 20, 2023

Choose a reason for hiding this comment

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

I think from a managed perspective it's all going to be a library.

@simonrozsival I would make a property similar to

<WasmGenerateAppBundle Condition="'$(WasmGenerateAppBundle)' == ''">true</WasmGenerateAppBundle>

Copy link
Member

Choose a reason for hiding this comment

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

We explicitly set the properties for the test projects:

<IsBrowserWasmProject Condition="'$(IsBrowserWasmProject)' == ''">true</IsBrowserWasmProject>
<WasmGenerateAppBundle Condition="'$(WasmGenerateAppBundle)' == ''">true</WasmGenerateAppBundle>

Copy link
Member Author

Choose a reason for hiding this comment

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

I replaced the _SkipAndroidGenerateAppBundle with AndroidGenerateAppBundle.

@lambdageek lambdageek requested a review from steveisok January 20, 2023 15:20
Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

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

This seems like an ok hack to me, but I'd like some feedback from someone who understands the android libraries test setup more than me.

It still doesn't make sense to me that src/FunctionalTests/TestAssets/**/*.csproj should even be considered for APK creation if it's not IsTestProject or IsFunctionalTest or anything like that.

@steveisok
Copy link
Member

This seems like an ok hack to me, but I'd like some feedback from someone who understands the android libraries test setup more than me.

It still doesn't make sense to me that src/FunctionalTests/TestAssets/**/*.csproj should even be considered for APK creation if it's not IsTestProject or IsFunctionalTest or anything like that.

It doesn't look like we gate much on these properties. On Android/iOS, IsFunctionalTest helps understand where to look for the apk's. I don't think it was ever about opting in.

@simonrozsival
Copy link
Member Author

/azp run runtime-android

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@simonrozsival simonrozsival merged commit 825323f into dotnet:main Jan 24, 2023
@simonrozsival simonrozsival deleted the android-skip-generating-app-bundle branch January 24, 2023 15:46
@ghost ghost locked as resolved and limited conversation to collaborators Feb 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants