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

[iOS][NativeAOT] Adding NativeAOT RunOniOS device test #19923

Merged
merged 2 commits into from Jan 29, 2024

Conversation

ivanpovazan
Copy link
Member

@ivanpovazan ivanpovazan commented Jan 16, 2024

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)
    var appFile = Path.Combine(projectDir, "bin", config, $"{framework}-ios", "iossimulator-x64", $"{Path.GetFileName(projectDir)}.app");
    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 #19817

/cc: @jonathanpeppers @simonrozsival

@mattleibow
Copy link
Member

Might need this to also have the AOT:

- name: $(iosTestsVmPool)

@ivanpovazan
Copy link
Member Author

Might need this to also have the AOT:

- name: $(iosTestsVmPool)

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.

@jonathanpeppers
Copy link
Member

@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?

@ivanpovazan
Copy link
Member Author

@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.

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.

LGTM!

image

@ivanpovazan
Copy link
Member Author

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

@jonathanpeppers jonathanpeppers enabled auto-merge (squash) January 26, 2024 15:48
@jonathanpeppers
Copy link
Member

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

@jonathanpeppers
Copy link
Member

/rebase

@ivanpovazan
Copy link
Member Author

ivanpovazan commented Jan 29, 2024

@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 ^

@jonathanpeppers jonathanpeppers merged commit 11a274b into dotnet:main Jan 29, 2024
47 checks passed
PureWeen pushed a commit that referenced this pull request Feb 8, 2024
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 #19817
@github-actions github-actions bot locked and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-testing Unit tests, device tests platform/iOS 🍎
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants