[tests] option to include/exclude categories from Test APKs

[jonathanpeppers]

In some QA needs to run tests on devices without a network connection,
so an option to disable tests that rely on the network in Mono.Android-Tests
is needed.

NUnit has the option to include or exclude tests based on the
CategoryAttribute. Various tests were decorated with
[Category("InetAccess")] that appeared to use the internet. To verify
this works, I ran the tests with my internet disabled. The test run for
Mono.Android-Tests went down from 106 to 84 tests, with no failures.

To exclude a category, on Windows:

    msbuild Xamarin.Android.sln /t:RunApkTests /p:ExcludeCategories=InetAccess

On MacOS/Linux:

    make run-apk-tests EXCLUDECATEGORIES=InetAccess

If you want to specify multiple categories, use : (colon) to delimit
each category. This delimiter is used for various values throughout
Xamarin.Android's build because it works well from the command line for both
MSBuild and make.

I also added the option specify Include as well, but it isn't particularly
useful for this case.

[dnfclas]

@jonathanpeppers,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by .NET Foundation. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot

[jonathanpeppers]

To match Mono, I need to change the category to InetAccess: mono/mono@9b4b6d6

[jonpryor]

To exclude a category, on Windows:

msbuild Xamarin.Android.sln /t:RunApkTests /p:Exclude=InetAccess

I'm not sure $(Include) and $(Exclude) are the "best" property names here; it's potentially ambiguous. It's a verb without a noun.

I'd suggest $(IncludeCategories) and $(ExcludeCategories).

Additionally, please update Documentation/DevelopmentTips.md with this documentation.

Finally, as this updates e.g. TestSuiteInstrumentation.GetExcludedCategories(), this PR should probably update all overrides so that they'll work as expected. For example, within Xamarin.Android.Bcl-Tests:

		protected override IEnumerable<string> GetExcludedCategories ()
		{
			return App.GetExcludedCategories ();
		}

We'll want Xamarin.Android.Bcl-Tests to follow suit, even if there are currently no tests that will make use of this functionality. When there are such tests, we'll get very confused if the $(ExcludeCategories) property doesn't work on Xamarin.Android.Bcl-Tests, so we should fix that now, while we're thinking of it:

		protected override IEnumerable<string> GetExcludedCategories ()
		{
			var external = base.GetExcludedCategories ();
			return external != null
				? external.Concat (App.GetExcludedCategories ())
				: App.GetExternalCategories ();
		}

Writing the above makes me think that we should also update the semantics of GetExcludedCategories() and GetIncludedCategories() so that they never return null (because ugh).

[jonathanpeppers]

Instead of returning null, these will be the equivalent of yield break

[jonpryor]

TestSuiteActivity should also be updated to not return null.

[jonathanpeppers]

We need to check Any(), because an empty IEnumerable should also be skipped

[jonpryor]

It should arguably be skipped, but it means that categories will be enumerated multiple times, which is potentially problematic.

It's probably better to not use the .Any(), and have an alternate check.

[jonpryor]

This construct means that you can't use both $(IncludeCategories) and $(ExcludeCategories) at the same time:

msbuild Xamarin.Android.sln /t:RunApkTests /p:ExcludeCategories=InetAccess /p:IncludeCategories=Whatever

I'm not immediately sure how useful such a thing is, but if we're going to have $(IncludeCategories) at all, then we should sensibly support both...

I think that this will work sanely:

<PropertyGroup>
  <_IncludeCats Condition=" '$(IncludeCategories)' != '' ">include=$(IncludeCategories)</_IncludeCats>
  <_ExcludeCats Condition=" '$(ExcludeCategories)' != '' ">exclude=$(ExcludeCategories)</_ExcludeCats >
</PropertyGroup>
...
<RunInstrumentationTests
    InstrumentationArguments="$(_IncludeCats);$(_ExcludeCats)"
    ...
/>

[jonathanpeppers]

Yeah, this is better: it lets you specify include/exclude at the same time.

My example that worked:

make run-apk-tests INCLUDECATEGORIES=InetAccess EXCLUDECATEGORIES=Bologna

Of course no category is named Bologna, but the command in the log was:

Tool /Users/jonathanpeppers/android-toolchain/sdk/platform-tools/adb execution started with arguments:   shell am instrument  -e "include" "InetAccess" -e "exclude" "Bologna" -w "Mono.Android_Tests/xamarin.android.runtimetests.TestInstrumentation"

[jonpryor]

Should , be used as a delimiter?

The problem is, it can't be used on the command-line:

$ msbuild /nologo Xamarin.Android.sln /t:RunApkTests /p:ExcludeCategories=InetAccess,Bologna
MSBUILD : error MSB1006: Property is not valid.
Switch: Bologna

For switch syntax, type "MSBuild /help"

This is why : was chosen as the separator char for e.g. $(AndroidSupportedTargetJitAbis), as it can be specified on the command-line.