From cf72b0d93086d0e7e8dc7f03298746c07c0f5892 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Tue, 10 Oct 2017 17:59:42 -0400 Subject: [PATCH] [tests] Add Configuration info to test names Context: 0077d151 Context: d1d9820a A "funny" thing happened when commit e9daf5ea didn't build on Jenkins: I realized that not all tests were run in all configurations. From commit d1d9820a: > Why are some tests Debug-only and some aren't? The answer: time, primarily. Why run tests multiple times, when they can be potentially time-consuming? While tests can be slow, they're not always *that* slow -- except for `Xamarin.Android.Build.Tests` and the BCL tests -- and even there, program behavior can alter between Debug and Release configurations. See in particular commit 0077d151, in which the BCL tests are run only in the Debug configuration because tests *failed* when run in the Release configuration. The desire, then, is to run *all* tests in both Debug and Release configurations. Yes, it'll take longer! So what! (Within reason: `Xamarin.Android.Build.Tests` should only be run once!) However, this raises two problems: 1. Filename collisions 2. Jenkins unit test display Until now, all tests wrote files into a filename that didn't include the Configuration, e.g. `TestResult-Mono.Android_Tests.xml`. If we did run these tests twice, the second test invocation would overwrite the first test invocation. This isn't desirable. Then there's the display on Jenkins: if we did have e.g. `TestResult-Mono.Android_Tests-Debug.xml` and `TestResult-Mono.Android_Tests-Release.xml`, how will Jenkins display that information? I haven't tested, but I would assume that one of two things will occur, assuming reasonable Jenkins behavior: 1. Each test will be listed twice, e.g. ApplicationContextIsApp ApplicationContextIsApp 2. They'll be "merged" into a single entry. Neither of these behaviors is desirable: if Debug passes but Release fails, we need to be able to differentiate between them. Neither of these possible renderings allows us to tell which configuration fails. Solve both of these problems by introducing a new `` task. This task takes three values of importance: ```xml ``` The `` task will read in `SOURCE`, and if `SOURCE` is an XML file which we determine is NUnit2-formatted XML (root element of ``), we will update every `//test-case/@name` value so that it ends with ` / CONFIGURATION`. The updated XML is then written to the `DESTINATION` directory, with a filename that contains `CONFIGURATION`, and `SOURCE` is deleted. Thus, if we have a Debug-configuration `TestResult-Mono.Android_Tests.xml` file with XML fragment: ```xml ``` then `` will create the file `TestResult-Mono.Android_Tests-Debug.xml` file with XML fragment: ```xml ``` This allows us to run tests in both Debug and Release configurations while not inadvertently overwriting the `TestResults*.xml` files that Jenkins reads, and ensuring that the Jenkins test result output is rendered in a meaningfully useful fashion. Aside: when updating `//test-case/@name`, the resulting value *cannot* end in `)`. If it does, then the `(root)` package name issue fixed in commit 23b2642e reappears for the `generator` unit tests. **Completely random aside about the state of `xbuild`**: A development version of `` was "saner", using `ITaskItem[]` and not string: ```csharp partial class RenameTestCases { public ITaskItem[] SourceFiles {get; set;} // vs. // public string SourceFile {get; set;} } ``` The problem is that the above, while entirely reasonable, did not work at all correctly with `xbuild`: ```xml ``` Under `xbuild`, MSBuild properties would not be expanded, e.g. `RenameTestCases.SourceFiles` would get a "bizarro" value of e.g. `$(OutputPath)Mono.Android_Tests-Signed.apk`, which is *useless* and would result in `FileNotFoundException`s. MSBuild proper, of course, worked as desired. TODO: Once this is merged, update the Jenkins Configuration page so that instead of: make run-all-tests V=1 || exit 1 it instead runs both Debug and Release configuration tests: make run-all-tests SKIP_NUNIT_TESTS=1 V=1 || exit 1 make run-all-tests CONFIGURATION=Release V=1 || exit 1 Note that `$(SKIP_NUNIT_TESTS)` is specified so that we only run the lengthy (1+hr!) `Xamarin.Android.Build.Tests` tests in the Release configuration, not the Debug + Release configurations. --- Makefile | 20 +++-- build-tools/scripts/TestApks.targets | 39 ++++++++ .../Test/Mono.Android-Tests.projitems | 23 +++-- ...amarin.Android.Tools.BootstrapTasks.csproj | 2 + .../RenameTestCases.cs | 88 +++++++++++++++++++ .../Xamarin.Android.JcwGen-Tests.projitems | 2 +- tests/RunApkTests.targets | 9 +- .../Xamarin.Android.Bcl-Tests.csproj | 2 +- .../Xamarin.Android.Bcl-Tests.projitems | 2 +- .../Xamarin.Android.Bcl-Tests.targets | 1 + ...ms.Performance.Integration.Droid.projitems | 2 +- .../Xamarin.Android.Locale-Tests.projitems | 2 +- 12 files changed, 171 insertions(+), 21 deletions(-) create mode 100644 src/Xamarin.Android.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks/RenameTestCases.cs diff --git a/Makefile b/Makefile index a43837d4e50..7218f974e36 100644 --- a/Makefile +++ b/Makefile @@ -118,6 +118,11 @@ include tests/api-compatibility/api-compatibility.mk run-all-tests: run-nunit-tests run-ji-tests run-apk-tests run-api-compatibility-tests +rename-test-cases: + $(MSBUILD) $(MSBUILD_FLAGS) build-tools/scripts/TestApks.targets \ + /t:RenameTestCases /p:RenameTestCasesGlob="$(if $(RENAME_GLOB),$(RENAME_GLOB),`pwd`/TestResult-\*.xml)" \ + $(if $(KEEP_TEST_SOURCES),/p:DeleteTestCaseSourceFiles=False) + clean: $(MSBUILD) $(MSBUILD_FLAGS) /t:Clean Xamarin.Android.sln tools/scripts/xabuild $(MSBUILD_FLAGS) /t:Clean Xamarin.Android-Tests.sln @@ -140,14 +145,18 @@ define RUN_NUNIT_TEST if [ -f "bin/Test$(CONFIGURATION)/TestOutput-$(basename $(notdir $(1))).txt" ] ; then \ cat bin/Test$(CONFIGURATION)/TestOutput-$(basename $(notdir $(1))).txt ; \ fi + $(MAKE) rename-test-cases RENAME_GLOB="`pwd`/TestResult-$(basename $(notdir $(1))).xml" endef run-nunit-tests: $(NUNIT_TESTS) +ifneq ($(SKIP_NUNIT_TESTS),) $(foreach t,$(NUNIT_TESTS), $(call RUN_NUNIT_TEST,$(t),1)) +endif # $(SKIP_NUNIT_TESTS) == '' run-ji-tests: $(MAKE) -C "$(call GetPath,JavaInterop)" CONFIGURATION=$(CONFIGURATION) all ANDROID_SDK_PATH="$(call GetPath,AndroidSdk)" $(MAKE) -C "$(call GetPath,JavaInterop)" CONFIGURATION=$(CONFIGURATION) run-all-tests || true + $(MAKE) rename-test-cases RENAME_GLOB='"$(call GetPath,JavaInterop)"/TestResult-*Tests.xml' cp "$(call GetPath,JavaInterop)"/TestResult-*.xml . # .apk files to test on-device need to: @@ -157,11 +166,11 @@ TEST_APK_PROJECTS = \ src/Mono.Android/Test/Mono.Android-Tests.csproj \ tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/Xamarin.Android.JcwGen-Tests.csproj \ tests/locales/Xamarin.Android.Locale-Tests/Xamarin.Android.Locale-Tests.csproj \ - tests/Xamarin.Android.Bcl-Tests/Xamarin.Android.Bcl-Tests.csproj + tests/Xamarin.Android.Bcl-Tests/Xamarin.Android.Bcl-Tests.csproj \ + tests/Xamarin.Forms-Performance-Integration/Droid/Xamarin.Forms.Performance.Integration.Droid.csproj -TEST_APK_PROJECTS_RELEASE = \ +TEST_APK_PROJECTS_AOT = \ src/Mono.Android/Test/Mono.Android-Tests.csproj \ - tests/Xamarin.Forms-Performance-Integration/Droid/Xamarin.Forms.Performance.Integration.Droid.csproj # Syntax: $(call BUILD_TEST_APK,path/to/project.csproj,additional_msbuild_flags) define BUILD_TEST_APK @@ -177,7 +186,6 @@ endef run-apk-tests: $(call RUN_APK_TESTS, $(TEST_APK_PROJECTS), ) -ifneq ($(wildcard bin/Release),) - $(call RUN_APK_TESTS, $(TEST_APK_PROJECTS_RELEASE), /p:Configuration=Release) - $(call RUN_APK_TESTS, $(TEST_APK_PROJECTS_RELEASE), /p:Configuration=Release /p:AotAssemblies=true) +ifeq ($(CONFIGURATION),Release) + $(call RUN_APK_TESTS, $(TEST_APK_PROJECTS_AOT), /p:AotAssemblies=true) endif diff --git a/build-tools/scripts/TestApks.targets b/build-tools/scripts/TestApks.targets index 589d0cbfb86..7d22730d651 100644 --- a/build-tools/scripts/TestApks.targets +++ b/build-tools/scripts/TestApks.targets @@ -3,6 +3,7 @@ + @@ -183,4 +184,42 @@ AddResults="true" Activity="%(TestApk.Activity)" /> + + + + + <_DeleteSource Condition=" '$(DeleteTestCaseSourceFiles)' != '' ">$(DeleteTestCaseSourceFiles) + <_DeleteSource Condition=" '$(_DeleteSource)' == '' ">True + + + <_RenameSource1 Include="$(RenameTestCasesGlob)" /> + + + <_RenameSource Include="%(_RenameSource1.Identity)"> + @(_RenameSource1->'%(RootDir)%(Directory)') + + + + + + + diff --git a/src/Mono.Android/Test/Mono.Android-Tests.projitems b/src/Mono.Android/Test/Mono.Android-Tests.projitems index 13bc4d032f6..4f4c051164a 100644 --- a/src/Mono.Android/Test/Mono.Android-Tests.projitems +++ b/src/Mono.Android/Test/Mono.Android-Tests.projitems @@ -2,7 +2,7 @@ - <_MonoAndroidTestResultsPath>$(MSBuildThisFileDirectory)..\..\..\TestResult-Mono.Android_Tests.xml + <_MonoAndroidTestResultsPath>$(OutputPath)TestResult-Mono.Android_Tests.xml <_MonoAndroidTestPackage>Mono.Android_Tests <_MonoAndroidTestApkFile>$(OutputPath)Mono.Android_Tests-Signed.apk <_MonoAndroidTestApkSizesInput>apk-sizes-$(_MonoAndroidTestPackage)-$(Configuration)$(_AotName).txt @@ -16,10 +16,21 @@ - - - - - + + + + diff --git a/src/Xamarin.Android.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks.csproj b/src/Xamarin.Android.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks.csproj index d043c7f0aa0..3def463d7a5 100644 --- a/src/Xamarin.Android.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks.csproj +++ b/src/Xamarin.Android.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks.csproj @@ -35,6 +35,7 @@ + @@ -47,6 +48,7 @@ + diff --git a/src/Xamarin.Android.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks/RenameTestCases.cs b/src/Xamarin.Android.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks/RenameTestCases.cs new file mode 100644 index 00000000000..ab0ceffdfde --- /dev/null +++ b/src/Xamarin.Android.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks/RenameTestCases.cs @@ -0,0 +1,88 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Text; +using System.Xml.Linq; + +using Microsoft.Build.Framework; +using Microsoft.Build.Utilities; + +namespace Xamarin.Android.Tools.BootstrapTasks +{ + public class RenameTestCases : Task + { + public bool DeleteSourceFiles { get; set; } + public string Configuration { get; set; } + [Required] + public string SourceFile { get; set; } + [Required] + public string DestinationFolder { get; set; } + + [Output] + public ITaskItem[] CreatedFiles { get; set; } + + public override bool Execute () + { + Log.LogMessage (MessageImportance.Low, $"Task {nameof (RenameTestCases)}"); + Log.LogMessage (MessageImportance.Low, $" {nameof (Configuration)}: {Configuration}"); + Log.LogMessage (MessageImportance.Low, $" {nameof (DeleteSourceFiles)}: {DeleteSourceFiles}"); + Log.LogMessage (MessageImportance.Low, $" {nameof (DestinationFolder)}: {DestinationFolder}"); + Log.LogMessage (MessageImportance.Low, $" {nameof (SourceFile)}: {SourceFile}"); + + var createdFiles = new List (); + var testNameSuffix = string.IsNullOrWhiteSpace (Configuration) + ? "" + : $" / {Configuration}"; + try { + var dest = FixupTestResultFile (SourceFile, testNameSuffix); + var item = new TaskItem (dest); + item.SetMetadata ("SourceFile", SourceFile); + createdFiles.Add (item); + } + catch (Exception e) { + Log.LogErrorFromException (e); + } + + CreatedFiles = createdFiles.ToArray (); + + Log.LogMessage (MessageImportance.Low, $" [Output] {nameof (CreatedFiles)}:"); + foreach (var f in CreatedFiles) { + Log.LogMessage (MessageImportance.Low, $" [Output] {f}:"); + } + + return !Log.HasLoggedErrors; + } + + string FixupTestResultFile (string source, string testNameSuffix) + { + var doc = XDocument.Load (source); + switch (doc.Root.Name.LocalName) { + case "test-results": + FixupNUnit2Results (doc, testNameSuffix); + break; + } + var destFilename = Path.GetFileNameWithoutExtension (source) + + (string.IsNullOrWhiteSpace (Configuration) ? "" : "-" + Configuration) + + Path.GetExtension (source); + var dest = Path.Combine (DestinationFolder, destFilename); + + doc.Save (dest); + if (DeleteSourceFiles && Path.GetFullPath (source) != Path.GetFullPath (dest)) { + File.Delete (source); + } + return dest; + } + + void FixupNUnit2Results (XDocument doc, string testNameSuffix) + { + foreach (var e in doc.Descendants ("test-case")) { + var name = (string) e.Attribute ("name"); + if (name.EndsWith (testNameSuffix, StringComparison.OrdinalIgnoreCase)) + continue; + name += testNameSuffix; + e.SetAttributeValue ("name", name); + } + } + } +} diff --git a/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/Xamarin.Android.JcwGen-Tests.projitems b/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/Xamarin.Android.JcwGen-Tests.projitems index ea258a6242c..54fdc806cc4 100644 --- a/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/Xamarin.Android.JcwGen-Tests.projitems +++ b/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/Xamarin.Android.JcwGen-Tests.projitems @@ -4,7 +4,7 @@ Xamarin.Android.JcwGen_Tests xamarin.android.jcwgentests.TestInstrumentation - $(MSBuildThisFileDirectory)..\..\..\TestResult-Xamarin.Android.JcwGen_Tests.xml + $(OutputPath)TestResult-Xamarin.Android.JcwGen_Tests.xml $(MSBuildThisFileDirectory)..\..\..\build-tools\scripts\TimingDefinitions.txt diff --git a/tests/RunApkTests.targets b/tests/RunApkTests.targets index 96f2601d7fb..7c37121dc88 100644 --- a/tests/RunApkTests.targets +++ b/tests/RunApkTests.targets @@ -19,9 +19,9 @@ `$(TEST_APK_PROJECTS_RELEASE)` within the toplevel `Makefile`. --> - - - + + + @@ -30,7 +30,8 @@ UndeployTestApks; DeployTestApks; RunTestApks; - ReleaseAndroidTarget + ReleaseAndroidTarget; + RenameApkTestCases; true - pdbonly + Full true ..\..\bin\TestRelease prompt diff --git a/tests/Xamarin.Android.Bcl-Tests/Xamarin.Android.Bcl-Tests.projitems b/tests/Xamarin.Android.Bcl-Tests/Xamarin.Android.Bcl-Tests.projitems index 6ae441b13d9..4aaab1082c5 100644 --- a/tests/Xamarin.Android.Bcl-Tests/Xamarin.Android.Bcl-Tests.projitems +++ b/tests/Xamarin.Android.Bcl-Tests/Xamarin.Android.Bcl-Tests.projitems @@ -4,7 +4,7 @@ Xamarin.Android.Bcl_Tests xamarin.android.bcltests.TestInstrumentation - $(MSBuildThisFileDirectory)..\..\TestResult-Xamarin.Android.Bcl_Tests.xml + $(OutputPath)TestResult-Xamarin.Android.Bcl_Tests.xml $(MSBuildThisFileDirectory)..\..\build-tools\scripts\TimingDefinitions.txt diff --git a/tests/Xamarin.Android.Bcl-Tests/Xamarin.Android.Bcl-Tests.targets b/tests/Xamarin.Android.Bcl-Tests/Xamarin.Android.Bcl-Tests.targets index 63ff214601e..e17adc3597c 100644 --- a/tests/Xamarin.Android.Bcl-Tests/Xamarin.Android.Bcl-Tests.targets +++ b/tests/Xamarin.Android.Bcl-Tests/Xamarin.Android.Bcl-Tests.targets @@ -62,6 +62,7 @@ HostOS="$(HostOS)" DestinationFolder="..\..\bin\$(Configuration)\bcl-tests" /> + Xamarin.Forms_Performance_Integration Xamarin.Forms_Performance_Integration/md52b709e14dec302485bbcaeac0bd817ce.MainActivity - $(MSBuildThisFileDirectory)..\..\..\TestResult-Xamarin.Forms_Test.xml + $(MSBuildThisFileDirectory)timing-definitions.txt diff --git a/tests/locales/Xamarin.Android.Locale-Tests/Xamarin.Android.Locale-Tests.projitems b/tests/locales/Xamarin.Android.Locale-Tests/Xamarin.Android.Locale-Tests.projitems index 204e62abda4..ed300db7535 100644 --- a/tests/locales/Xamarin.Android.Locale-Tests/Xamarin.Android.Locale-Tests.projitems +++ b/tests/locales/Xamarin.Android.Locale-Tests/Xamarin.Android.Locale-Tests.projitems @@ -4,7 +4,7 @@ Xamarin.Android.Locale_Tests xamarin.android.localetests.TestInstrumentation - $(MSBuildThisFileDirectory)..\..\..\TestResult-Xamarin.Android.Locale_Tests.xml + $(OutputPath)TestResult-Xamarin.Android.Locale_Tests.xml $(MSBuildThisFileDirectory)..\..\..\build-tools\scripts\TimingDefinitions.txt