Skip to content

Add base entry point class for android tests#454

Merged
greenEkatherine merged 13 commits intodotnet:mainfrom
greenEkatherine:base-test-instrumentation
Mar 11, 2021
Merged

Add base entry point class for android tests#454
greenEkatherine merged 13 commits intodotnet:mainfrom
greenEkatherine:base-test-instrumentation

Conversation

@greenEkatherine
Copy link
Contributor

@greenEkatherine greenEkatherine commented Feb 16, 2021

Addressed #383

With this approach test result file could be retrieved only on devices with API <=29

In scope of #385 I will add ability to pass test result file for API 30 as well

@greenEkatherine greenEkatherine linked an issue Feb 16, 2021 that may be closed by this pull request
Create a "default" or "common" instrumentation base #383
Closed
@greenEkatherine greenEkatherine mentioned this pull request Feb 16, 2021
Closed
premun
premun reviewed Feb 17, 2021
View reviewed changes
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.

Couple of questions left. Also please consider describing the usage in the README

mandel-macaque
mandel-macaque suggested changes Feb 17, 2021
View reviewed changes
mattleibow
mattleibow reviewed Feb 18, 2021
View reviewed changes
Katya Sokolova and others added 2 commits February 18, 2021 17:26
@premun
Update src/Microsoft.DotNet.XHarness.InstrumentationBase.Xunit/Micros…
239a1cf 
…oft.DotNet.XHarness.InstrumentationBase.Xunit.csproj

Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>
review feedback part 1
abc732a
@greenEkatherine greenEkatherine force-pushed the base-test-instrumentation branch from 6668123 to abc732a Compare February 18, 2021 16:57
mandel-macaque
mandel-macaque suggested changes Feb 23, 2021
View reviewed changes
src/Microsoft.DotNet.XHarness.InstrumentationBase.Xunit/AndroidInstrumentationBase.cs Outdated
Comment on lines +41 to +46
protected virtual IEnumerable<Assembly> Tests
{
get
{
yield return Assembly.GetExecutingAssembly();
}
Copy link
Contributor

@mandel-macaque mandel-macaque Feb 23, 2021

Choose a reason for hiding this comment

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

I'm confuse by this property. Just returns the currently executing assembly. What is the goal? I'm not an android expert. But either:

  1. We have on instrumentation with several test assemblies (each one of them with several tests).
  2. We have an instrumentation class per assembly, with a number of tests.

If the second is correct, we could modify the property to be just Assembly, else.. I don't understand why the class returns a IEnumerable.

Copy link
Contributor Author

@greenEkatherine greenEkatherine Feb 26, 2021

Choose a reason for hiding this comment

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

This property was aimed to pass tests from user to TestsEntryPoint::GetTestAssemblies. Anyway I'm going to remove Instrumentation class from this pull request to adjust it with #480

@mandel-macaque
Update src/Microsoft.DotNet.XHarness.InstrumentationBase.Xunit/Androi…
0135939 
…dInstrumentationBase.cs

Co-authored-by: Manuel de la Pena <mandel@microsoft.com>
@greenEkatherine greenEkatherine linked an issue Feb 26, 2021 that may be closed by this pull request
Provide base TestEntryPoint for Android #480
Closed
premun
premun approved these changes Mar 10, 2021
View reviewed changes
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.

Looks clean!

@greenEkatherine greenEkatherine changed the title Add base instrumentation class for android tests Add base entry point class for android tests Mar 10, 2021
mattleibow
mattleibow reviewed Mar 10, 2021
View reviewed changes
Copy link
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

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

Quite good. Just a comment on the visibility of the args.

But I am also not sure how the filter from the CLI gets to the entry point. I added a --method and it never exists on any of the command lines generated. If the CLI runner is going to pass them, then maybe we need to coordinate with what the CLI will use?

@greenEkatherine
Copy link
Contributor Author

greenEkatherine commented Mar 11, 2021

But I am also not sure how the filter from the CLI gets to the entry point. I added a --method and it never exists on any of the command lines generated. If the CLI runner is going to pass them, then maybe we need to coordinate with what the CLI will use?

--class and --method parameters are filtering for apple commands only. I'm sorry for the confusion, there are some mix of arguments in help command now but it's going to be fixed in #431

However, TestRunner has more options to work with classes and methods filtering, so I added the possibility to use it in that way.

mattleibow
mattleibow reviewed Mar 11, 2021
View reviewed changes
@greenEkatherine greenEkatherine merged commit 303be48 into dotnet:main Mar 11, 2021
@greenEkatherine greenEkatherine deleted the base-test-instrumentation branch March 11, 2021 14:35
@mattleibow
Copy link
Member

mattleibow commented Mar 11, 2021

Very nice! Thanks for all of this!

premun added a commit that referenced this pull request Mar 17, 2021
@mandel-macaque @premun
@dotnet-maestro @lewing @radical
Merge main into xcode12.5 (#506)
62aa7fc 
* Add triggers for xcode/* branches (#450)

* Remove dead code around TestRunner argument passing (#449)

- I compared what we send to and what we receive from the TestRunner EntryPoint and noticed quite a bit of unused options. These are remnants of the Xamarin code with which we don't share the EntryPoint anymore
- We are passing arguments both via env variables and arguments so we will only pass the env vars now
- There was a bug in the definition of the `class` argument and it wasn't parsed properly
- Another thing I fixed are the "extra" arguments - I don't they they were working until know because we prepend them with a `-app-arg:` prefix which is only recognized in the Xamarin's mono Main

* Improve various logging (#463)

Side effects of other work, related to #358

* Fail faster when cannot find any devices/emulators (#469)

* fail faster when cannot find any devices/emulators

* Apply suggestions from code review

Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>

Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>

* Move to new location of netcorenativeassets (#471)

* Remove the DummyTestProject (#472)

* Add support for the MacCatalyst target (#464)

- Adds support for the `apple test -t=maccatalyst` command (resolves #435)
- Getting system logs for the process is not working at the moment (#459)
- The `apple run` command cannot detect exit code yet (#462)

* Capture System.log when running MacCatalyst apps (#474)

Resolves #459

* Make sure the MacCatalyst app's binary is executable (#475)

* Allow to skip device-id for android install/run/uninstall (#479)

* allow to skip device-id for android install/run/uninstall if device can be choosed unambiguously

* Update src/Microsoft.DotNet.XHarness.Android/AdbRunner.cs

Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>

* addressed review feedback

Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>

* Update dependencies from https://github.com/dotnet/arcade build 20210225.5 (#482)

[main] Update dependencies from dotnet/arcade

* Run MacCatalyst via `open w` and detect exit code from logs (#484)

Resolves #462

* Add integration tests for MacCatalyst (#487)

* Add integration tests for the MacCatalyst target
* Make Helix tests run on local
* Remove EnableXUnitReporter
* Create template proj for integration tests

* Display the name of the exit code (#488)

* Wait for MacCatalyst system.log flush (#490)

* Don't reset the stream if we got a partial message (#489)

Fixes #473

* Do not report LaunchFailure when the app connected over TCP (#491)

* [wasm] add `--expected-exit-code` for `wasm test-browser`, similar to (#492)

.. what `wasm test` has.

* Rename iOS namespace to Apple (#494)

* Merge SimulatorInstaller commands to main CLI (#497)

This removes the SimulatorInstaller project and moves the file to CLI so that you can do:
```
xharness apple simulators list
```

When installing the simulators, it doesn't accept the same format e.g. `ios-simulator-64` of targets as the CLI, this will be a follow-up.

#478

* [wasm] Ensure that Safari/firefox drivers are Disposed in case of (#500)

.. failures. This manifested when running with Safari, and it doesn't
have `remote automation` enabled. That would cause xharness to get
stuck because of the still running driver process.

```
crit: System.InvalidOperationException: Could not create a session: You must enable the 'Allow Remote Automation' option in Safari's Develop menu to control Safari via WebDriver. (SessionNotCreated)
         at OpenQA.Selenium.Remote.RemoteWebDriver.UnpackAndThrowOnError(Response errorResponse)
         at OpenQA.Selenium.Remote.RemoteWebDriver.Execute(String driverCommandToExecute, Dictionary`2 parameters)
         at OpenQA.Selenium.Remote.RemoteWebDriver.StartSession(ICapabilities desiredCapabilities)
         at OpenQA.Selenium.Remote.RemoteWebDriver..ctor(ICommandExecutor commandExecutor, ICapabilities desiredCapabilities)
         at OpenQA.Selenium.Safari.SafariDriver..ctor(SafariDriverService service, SafariOptions options, TimeSpan commandTimeout)
         at Microsoft.DotNet.XHarness.CLI.Commands.Wasm.WasmTestBrowserCommand.GetSafariDriver(ILogger logger) in /_/src/Microsoft.DotNet.XHarness.CLI/Commands/WASM/Browser/WasmTestBrowserCommand.cs:line 98
         at Microsoft.DotNet.XHarness.CLI.Commands.Wasm.WasmTestBrowserCommand.InvokeInternal(ILogger logger) in /_/src/Microsoft.DotNet.XHarness.CLI/Commands/WASM/Browser/WasmTestBrowserCommand.cs:line 63
         at Microsoft.DotNet.XHarness.Common.CLI.Commands.XHarnessCommand.Invoke(IEnumerable`1 arguments) in /_/src/Microsoft.DotNet.XHarness.Common/CLI/Commands/XHarnessCommand.cs:line 120
XHarness exit code: 71 (GENERAL_FAILURE)
XHarness artifacts: ./xharness-output
```

* [wasm][test-browser] Add `--browser-path` to use custom browser paths (#501)

* Detect old version of MacOS in MacCatalyst test runs (#495)

* Add base entry point class for android tests (#454)

* add base instrumentation class for android tests

* Update src/Microsoft.DotNet.XHarness.InstrumentationBase.Xunit/Microsoft.DotNet.XHarness.InstrumentationBase.Xunit.csproj

Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>

* review feedback 

* Update src/Microsoft.DotNet.XHarness.InstrumentationBase.Xunit/AndroidInstrumentationBase.cs

Co-authored-by: Manuel de la Pena <mandel@microsoft.com>

* addressed review feedback

* remove instrumentation part, leave only default test entry point for android

* address review feedback

* remove test project

Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>
Co-authored-by: Manuel de la Pena <mandel@microsoft.com>

* Fix CLI behavior when doing `xharness` or `xharness help` (#504)

* [main] Update dependencies from dotnet/arcade (#499)

[main] Update dependencies from dotnet/arcade


 - Merge remote-tracking branch 'origin/main' into darc-main-37ceeb7c-81af-4535-a6eb-96fcd2db6c28

 - Merge remote-tracking branch 'origin/main' into darc-main-37ceeb7c-81af-4535-a6eb-96fcd2db6c28

 - Update unit tests project to net6.0

* Fix possible merge mistake.

* Revert performance-setup.sh

Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>
Co-authored-by: Katya Sokolova <esokolov@microsoft.com>
Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Larry Ewing <lewing@microsoft.com>
Co-authored-by: Ankit Jain <radical@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jonathanpeppers jonathanpeppers jonathanpeppers left review comments

@mattleibow mattleibow mattleibow left review comments

@premun premun premun approved these changes

+1 more reviewer

@mandel-macaque mandel-macaque mandel-macaque approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Provide base TestEntryPoint for Android Create a "default" or "common" instrumentation base

5 participants

@greenEkatherine @mattleibow @jonathanpeppers @mandel-macaque @premun