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

Use XHarnessApkToTest to create helix work items for runtime tests when running on Android #57292

Merged
merged 28 commits into from
Sep 9, 2021

Conversation

fanyang-mono
Copy link
Member

No description provided.

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

Copy link
Member

@premun premun left a comment

Choose a reason for hiding this comment

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

This is the correct approach and what I hoped the solution might look like. Just beware that one thing will most likely break for Apple - you won't be able to read env variables, such as $HELIX_WORKITEM_UPLOAD_ROOT, anymore thanks to the launchctl workaround. So if the generated code calling XHarness uses these, it should switch over to variables defined by the Helix SDK (https://github.com/dotnet/arcade/tree/main/src/Microsoft.DotNet.Helix/Sdk/tools/xharness-runner#variables-defined-for-apple-scenarios)

src/tests/Common/helixpublishwitharcade.proj Outdated Show resolved Hide resolved
@SamMonoRT
Copy link
Member

@naricc is going to take a look at the suggested changes and take over this PR

@naricc
Copy link
Contributor

naricc commented Aug 19, 2021

@premun Hey, I have been asked to pickup this work, but I didn't really get to speak to Fan before she went on vacation. I've progressed it with the obvious steps so far, but I am not sure where to go.

I see that it is failing with HelixWorkItems is empty, but I assume the change removing the work items was done for a reason. Is this supposed to use another API that doesn't take HelixWorkItems? Is there a document I should be looking at for how CustomCommands is supposed to work?

@premun
Copy link
Member

premun commented Aug 20, 2021

@naricc the docs for the XHarness part of the Helix SDK are here: https://github.com/dotnet/arcade/blob/main/src/Microsoft.DotNet.Helix/Sdk/tools/xharness-runner/Readme.md

The goal is to define these XHarnessApkToTest items which the Helix SDK will turn into HelixWorkItem (and do some magic around those). You will also need the CustomCommands feature but I see Fan has done that already.

If you see XHarnessApkToTest items defined but not HelixWorkItem, then I can think of 2 possible reasons:

  1. You are not turning the feature on (see the How to use section) but I am not sure this is the case since we already include and use XHarness (I think)
  2. You are not defining these items well or you do but in some target that runs later than the Helix SDK targets run.

I checked the binlog of your build quickly and I can see you are not defining them at all probably:
image

@ghost
Copy link

ghost commented Aug 26, 2021

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details

null

Author: fanyang-mono
Assignees: -
Labels:

area-Infrastructure

Milestone: -

@fanyang-mono
Copy link
Member Author

Currently, blocked by dotnet/arcade#7808

@fanyang-mono
Copy link
Member Author

Currently, blocked by dotnet/arcade#7808

This issue has been resolved.

@fanyang-mono fanyang-mono marked this pull request as ready for review September 8, 2021 16:28
@fanyang-mono fanyang-mono changed the title Use CustomCommand to run runtime tests for mobile targets on helix Use XHarnessApkToTest to create helix work items for runtime tests when running on Android Sep 8, 2021
Copy link
Member

@akoeplinger akoeplinger left a comment

Choose a reason for hiding this comment

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

Looks good to me, though @premun is the expert here :)

src/tests/Common/helixpublishwitharcade.proj Show resolved Hide resolved
@fanyang-mono fanyang-mono merged commit ef85762 into dotnet:main Sep 9, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Oct 9, 2021
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.

7 participants