maui icon indicating copy to clipboard operation
maui copied to clipboard

[iOS][NativeAOT] Adding NativeAOT RunOniOS device test

Open ivanpovazan opened this issue 1 year ago • 8 comments

Description

This PR adds an integration test for testing NativeAOT running on iOS by extending the existing RunOniOS test method with two new parameters:

  • runtimeIdentifier - string denoting the target runtime identifier i.e., on which platform the test will be executed. Since currently CI only runs these tests on simulators (note: referring to iossimulator-x64 in the target path below) https://github.com/dotnet/maui/blob/f9a885219953dd67171b0986f9746795fc5fc207/src/TestUtils/src/Microsoft.Maui.IntegrationTests/AppleTemplateTests.cs#L47 we are passing the same value iossimulator-x64 in all cases for the time being.
  • runtimeVariant - new enum type - RuntimeVariant differentiating runtime variants to run the test with (at the moment supported variants are Mono and NativeAOT).

Contributes to https://github.com/dotnet/maui/issues/19817

/cc: @jonathanpeppers @simonrozsival

ivanpovazan avatar Jan 16 '24 13:01 ivanpovazan

Might need this to also have the AOT: https://github.com/dotnet/maui/blob/5727faa60692a5cc80876fa241cf47634c5f967d/eng/pipelines/common/maui-templates.yml#L43

mattleibow avatar Jan 16 '24 16:01 mattleibow

Might need this to also have the AOT:

https://github.com/dotnet/maui/blob/5727faa60692a5cc80876fa241cf47634c5f967d/eng/pipelines/common/maui-templates.yml#L43

Thanks. I wasn't aware only RunOniOS is running.

@mattleibow Would it make more sense to add an additional parameter (e.g., bool runWithNativeAOT) on a existing RunOniOS test method instead? The only complication will be the slightly different build properties in case of NativeAOT, but we avoid changing the .yml file.

ivanpovazan avatar Jan 16 '24 17:01 ivanpovazan

@ivanpovazan I think you could do either one, but maybe merging your test into the existing RunOniOS test is easier?

You could add a bool runWithNativeAOT parameter and set extra MSBuild properties when it's true?

jonathanpeppers avatar Jan 16 '24 20:01 jonathanpeppers

@ivanpovazan I think you could do either one, but maybe merging your test into the existing RunOniOS test is easier?

You could add a bool runWithNativeAOT parameter and set extra MSBuild properties when it's true?

I modified the RunOniOS test and updated the description of the PR according to the changes.

ivanpovazan avatar Jan 17 '24 10:01 ivanpovazan

The added test has passed: https://dev.azure.com/xamarin/public/_build/results?buildId=105493&view=logs&j=5577a1a6-5922-5c8f-f8aa-3ff6c6e2bbde&t=01eca571-4306-5a2f-af0e-e1e91b20d32a&l=708

ivanpovazan avatar Jan 17 '24 14:01 ivanpovazan

@jonathanpeppers can we merged this, or should we retry to get the CI green?

ivanpovazan avatar Jan 26 '24 13:01 ivanpovazan

I clicked retry one more time, we might need to /rebase as they fixed some CI things in main recently.

jonathanpeppers avatar Jan 26 '24 15:01 jonathanpeppers

/rebase

jonathanpeppers avatar Jan 26 '24 21:01 jonathanpeppers

@jonathanpeppers I manually rebased and pushed, but I see it can be done with /rebase as well.

~~In any case, it didn't help to get green CI on this PR~~

UPDATE: CI finally passed on another retry ^

ivanpovazan avatar Jan 29 '24 09:01 ivanpovazan