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

Introduce install, run-test, uninstall args for Android testing command #397

Closed
fanyang-mono opened this issue Dec 10, 2020 · 7 comments · Fixed by #429
Closed

Introduce install, run-test, uninstall args for Android testing command #397

fanyang-mono opened this issue Dec 10, 2020 · 7 comments · Fixed by #429
Assignees
Labels
android Android area enhancement New feature or request

Comments

@fanyang-mono
Copy link
Member

I was working on enabling runtime test to run on Android devices. And the current runtime test infrastructure calls the xharness Android testing command once per Fact. That means the same Android app got installed and uninstalled multiple times per Xunit test file, which slows down the test dramatically.

It would be nice to add an arg to tell the Android testing command to only install, uninstall or run-test. That way, the Android app only needs to install/uninstall once per test file in the initialize/cleanup methods.

cc: @akoeplinger @MattGal

@MattGal
Copy link
Member

MattGal commented Dec 10, 2020

@fanyang-mono can you help me understand (if this is the case) why it is not feasible to change the behavior of the runtime coreclr tests? They've been the same "one executable per test case" since the early 2000s but the only reason they seem to be this way is inertia and resistance to just refactoring them the way CoreFX did years ago.

@fanyang-mono
Copy link
Member Author

@fanyang-mono can you help me understand (if this is the case) why it is not feasible to change the behavior of the runtime coreclr tests? They've been the same "one executable per test case" since the early 2000s but the only reason they seem to be this way is inertia and resistance to just refactoring them the way CoreFX did years ago.

TBH, I am not 100% sure about the history of runtime tests, since I just started to work on it recently. But you probably are right that runtime tests need to be refactored to better fit the xharness test framework.

@trylek Could you please provide some comments on Matt's question?

@trylek
Copy link
Member

trylek commented Dec 10, 2020

I agree that having multiple tests per executable would speed up testing and be a general win on multiple fronts, in fact I mentioned that myself recently on an issue thread. The complicated part is how to get to that state and it's certainly no short term project for multiple reasons. In contrast to library tests CoreCLR includes several groups of tests with vastly different characteristics. Interop tests, for instance, often require loading extra native components; JIT and typesystem tests include tons of tests written in IL that are non-trivial (require manual adjustments) to roll into a single assembly; some tests do weird things altogether (e.g. Crossgen resiliency tests running the compiler twice on slightly different versions of the code and doing additional magic). Long story short - thumbs up for bringing this up and I'm looking forward to the world with 100 CoreCLR test assemblies running 100 test cases each (accounting for today ~10K Pri1 tests) and taking 2 minutes total; but I don't see how to get to that state in less than a year at least.

@fanyang-mono
Copy link
Member Author

Could we please have this issue assigned and scheduled to be worked on soon? Currently, there are only 20% runtime tests running on Android devices. The coverage is quite low.

@MattGal @akoeplinger

@premun
Copy link
Member

premun commented Jan 11, 2021

cc @tkapin @greenEkatherine

@premun premun added android Android area enhancement New feature or request labels Jan 12, 2021
@greenEkatherine greenEkatherine self-assigned this Jan 13, 2021
@greenEkatherine
Copy link
Contributor

Sorry for the delay, the XHarness project was changing owners and we are now ready to look into this.

@greenEkatherine
Copy link
Contributor

I'm planning to make a follow-up pull request to add tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android Android area enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants
@premun @MattGal @trylek @greenEkatherine @fanyang-mono and others