From c859ad06668486ba87c741d4ee5dbd48fa73ed34 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Tue, 30 Nov 2021 12:35:47 -0500 Subject: [PATCH 1/6] [java-source-utils] Fix regresion from 77c9c5fa MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: 77c9c5fa617f50817eda3d358e881f437165c07d Commit 77c9c5fa introduced a regression in `tools/java-source-utils`, wherein the unit tests would fail, e.g. org.junit.ComparisonFailure: ../../../com/xamarin/JavaType.java Javadoc XML expected:<...um"> [ … but was:<...um"> [ Part of the problem is that this failure was *overlooked*; it's very common for PR builds to fail because mono SIGSEGVs, and we don't always take the time to restart failed PR builds to ensure that they're green (oops). The Windows builds, meanwhile, were green. Consequently, when commit 77c9c5fa introduced a test failure in the `java-source-utils` unit tests, but we didn't see the failure. This "lack of seeing" is in part because we've become inured to test failures (mono SIGSEGVs), but also because the `java-source-utils` unit tests: 1. Are *only* run on the `mac_build` job, and 2. The results of the `java-source-utils` unit tests are not shown in DevOps' **Tests** tab. The only way to see the failure is to read the full **Mac - Mono** > **Run Tests** output, and know what you're looking for. Fix these two more fundamental issues: 1. Update `tools/java-source-utils/build.gradle` so that when `gradle test` is run, JUnit XML-formatted test result files are created. 2. Update `core-tests.yaml` so that the `java-source-utils` unit tests are executed. 3. Publish the XML files created between (1) and (2). This should ensure that future `java-source-utils` failures are appropriately seen and not ignored. I then ran a PR build without fixing anything, verifying that the **Tests** tab now shows the expected unit tests failures. Fix the `java-source-utils` unit test expected output, so that the `java-source-utils` unit tests pass once again…on macOS. They don't current pass on Windows, for currently unknown reasons (but probably line ending-related). We will enable Windows support later. --- build-tools/automation/azure-pipelines.yaml | 15 +++------------ build-tools/automation/templates/core-tests.yaml | 16 ++++++++++++++++ .../templates/publish-test-results.yaml | 14 ++++++++++++++ build-tools/scripts/RunNUnitTests.targets | 5 +++++ tools/java-source-utils/build.gradle | 6 ++++++ .../src/test/resources/UnresolvedTypes.txt | 2 +- .../src/test/resources/UnresolvedTypes.xml | 4 ++-- .../com/microsoft/android/JavaType.params.txt | 2 +- .../resources/com/microsoft/android/JavaType.xml | 4 ++-- 9 files changed, 50 insertions(+), 18 deletions(-) create mode 100644 build-tools/automation/templates/publish-test-results.yaml diff --git a/build-tools/automation/azure-pipelines.yaml b/build-tools/automation/azure-pipelines.yaml index 510b27fd2..ea02853b2 100644 --- a/build-tools/automation/azure-pipelines.yaml +++ b/build-tools/automation/azure-pipelines.yaml @@ -70,12 +70,7 @@ jobs: msbuildArguments: /p:TestAssembly="bin\Test$(Build.Configuration)\generator-Tests.dll;bin\Test$(Build.Configuration)\Java.Interop.Tools.JavaCallableWrappers-Tests.dll;bin\Test$(Build.Configuration)\logcat-parse-Tests.dll;bin\Test$(Build.Configuration)\Xamarin.Android.Tools.ApiXmlAdjuster-Tests.dll;bin\Test$(Build.Configuration)\Xamarin.Android.Tools.Bytecode-Tests.dll;bin\Test$(Build.Configuration)\Java.Interop.Tools.Generator-Tests.dll;bin\Test$(Build.Configuration)\Xamarin.SourceWriter-Tests.dll" condition: succeededOrFailed() - - task: PublishTestResults@2 - displayName: Publish Test Results - inputs: - testResultsFormat: NUnit - testResultsFiles: TestResult-*.xml - condition: succeededOrFailed() + - template: templates\publish-test-results.yaml - job: windows_dotnet_build displayName: Windows - .NET Core @@ -145,12 +140,7 @@ jobs: exit $r displayName: Run Tests - - task: PublishTestResults@2 - displayName: Publish Test Results - inputs: - testResultsFormat: NUnit - testResultsFiles: TestResult-*.xml - condition: succeededOrFailed() + - template: templates\publish-test-results.yaml - task: CopyFiles@2 displayName: 'Copy Files to: Artifact Staging Directory' @@ -189,5 +179,6 @@ jobs: - template: templates\core-tests.yaml parameters: runNativeTests: true + runJavaTests: true - template: templates\fail-on-issue.yaml diff --git a/build-tools/automation/templates/core-tests.yaml b/build-tools/automation/templates/core-tests.yaml index 5123234f6..5403c2de1 100644 --- a/build-tools/automation/templates/core-tests.yaml +++ b/build-tools/automation/templates/core-tests.yaml @@ -1,6 +1,7 @@ parameters: condition: succeeded() runNativeTests: false + runJavaTests: false steps: - task: DotNetCoreCLI@2 @@ -106,3 +107,18 @@ steps: command: test arguments: bin/Test$(Build.Configuration)$(NetCoreTargetFrameworkPathSuffix)/Java.Interop-PerformanceTests.dll continueOnError: true + +- task: DotNetCoreCLI@2 + displayName: 'Tests: java-source-utils' + condition: eq('${{ parameters.runJavaTests }}', 'true') + inputs: + command: build + arguments: tools/java-source-utils/java-source-utils.csproj /t:RunTests + continueOnError: true + +- task: PublishTestResults@2 + displayName: Publish JUnit Test Results + inputs: + testResultsFormat: JUnit + testResultsFiles: 'tools/java-source-utils/build/test-results/**/TEST-*.xml' + condition: succeededOrFailed() diff --git a/build-tools/automation/templates/publish-test-results.yaml b/build-tools/automation/templates/publish-test-results.yaml new file mode 100644 index 000000000..6a771acfe --- /dev/null +++ b/build-tools/automation/templates/publish-test-results.yaml @@ -0,0 +1,14 @@ +steps: +- task: PublishTestResults@2 + displayName: Publish NUnit Test Results + inputs: + testResultsFormat: NUnit + testResultsFiles: TestResult-*.xml + condition: succeededOrFailed() + +- task: PublishTestResults@2 + displayName: Publish JUnit Test Results + inputs: + testResultsFormat: JUnit + testResultsFiles: '**/TEST-*.xml' + condition: succeededOrFailed() diff --git a/build-tools/scripts/RunNUnitTests.targets b/build-tools/scripts/RunNUnitTests.targets index 0aac10147..ab169fe54 100644 --- a/build-tools/scripts/RunNUnitTests.targets +++ b/build-tools/scripts/RunNUnitTests.targets @@ -29,5 +29,10 @@ WorkingDirectory="$(_TopDir)" ContinueOnError="ErrorAndContinue" /> + \ No newline at end of file diff --git a/tools/java-source-utils/build.gradle b/tools/java-source-utils/build.gradle index 32b040e61..4fff4b577 100644 --- a/tools/java-source-utils/build.gradle +++ b/tools/java-source-utils/build.gradle @@ -54,3 +54,9 @@ jar { } archiveName 'java-source-utils.jar' } + +test { + reports { + junitXml.enabled = true + } +} diff --git a/tools/java-source-utils/src/test/resources/UnresolvedTypes.txt b/tools/java-source-utils/src/test/resources/UnresolvedTypes.txt index 89ce7c48d..4988cc9b6 100644 --- a/tools/java-source-utils/src/test/resources/UnresolvedTypes.txt +++ b/tools/java-source-utils/src/test/resources/UnresolvedTypes.txt @@ -4,7 +4,7 @@ public class UnresolvedTypes { /** * Method using unresolvable types. As such, we make do. * - * JNI Sig: method.(L.*example.name.UnresolvedParameterType;)L.*UnresolvedReturnType; + * JNI Sig: method.([L.*example.name.UnresolvedParameterType;)L.*UnresolvedReturnType; */ public static UnresolvedReturnType method(example.name.UnresolvedParameterType... parameter) { } diff --git a/tools/java-source-utils/src/test/resources/UnresolvedTypes.xml b/tools/java-source-utils/src/test/resources/UnresolvedTypes.xml index 3fac8937a..89762f592 100644 --- a/tools/java-source-utils/src/test/resources/UnresolvedTypes.xml +++ b/tools/java-source-utils/src/test/resources/UnresolvedTypes.xml @@ -2,11 +2,11 @@ - + +JNI Sig: method.([L.*example.name.UnresolvedParameterType;)L.*UnresolvedReturnType;]]> diff --git a/tools/java-source-utils/src/test/resources/com/microsoft/android/JavaType.params.txt b/tools/java-source-utils/src/test/resources/com/microsoft/android/JavaType.params.txt index e49e266d0..6e529d545 100644 --- a/tools/java-source-utils/src/test/resources/com/microsoft/android/JavaType.params.txt +++ b/tools/java-source-utils/src/test/resources/com/microsoft/android/JavaType.params.txt @@ -10,7 +10,7 @@ package com.xamarin func(java.lang.String[] values) instanceActionWithGenerics(T value1, E value2) staticActionWithGenerics(T value1, TExtendsNumber value2, java.util.List unboundedList, java.util.List extendsList, java.util.List superList) - sum(int first, int remaining) + sum(int first, int... remaining) class JavaType.ASC class JavaType.PSC class JavaType.RNC diff --git a/tools/java-source-utils/src/test/resources/com/microsoft/android/JavaType.xml b/tools/java-source-utils/src/test/resources/com/microsoft/android/JavaType.xml index ff472f1c1..873dc42be 100644 --- a/tools/java-source-utils/src/test/resources/com/microsoft/android/JavaType.xml +++ b/tools/java-source-utils/src/test/resources/com/microsoft/android/JavaType.xml @@ -127,9 +127,9 @@ - + - + From e3236936a9ab2cf15910ced2f74e1d13588c970c Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Tue, 30 Nov 2021 21:55:19 -0500 Subject: [PATCH 2/6] icanhaz diag logs? --- Makefile | 2 +- build-tools/automation/templates/core-build.yaml | 2 +- build-tools/automation/templates/core-tests.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index f44a1658f..ecbacacd5 100644 --- a/Makefile +++ b/Makefile @@ -147,7 +147,7 @@ run-ptests: $(PTESTS) bin/Test$(CONFIGURATION)/$(JAVA_INTEROP_LIB) exit $$r; run-java-source-utils-tests: - $(MSBUILD) $(MSBUILD_FLAGS) tools/java-source-utils/java-source-utils.csproj /t:RunTests + $(MSBUILD) $(MSBUILD_FLAGS) tools/java-source-utils/java-source-utils.csproj /t:RunTests -v:diag bin/Test$(CONFIGURATION)/$(JAVA_INTEROP_LIB): bin/$(CONFIGURATION)/$(JAVA_INTEROP_LIB) cp $< $@ diff --git a/build-tools/automation/templates/core-build.yaml b/build-tools/automation/templates/core-build.yaml index 1f47df088..db4a3f5b8 100644 --- a/build-tools/automation/templates/core-build.yaml +++ b/build-tools/automation/templates/core-build.yaml @@ -6,7 +6,7 @@ steps: displayName: Prepare Solution inputs: projects: Java.Interop.sln - arguments: '-c $(Build.Configuration) -target:Prepare -p:MaxJdkVersion=$(MaxJdkVersion)' + arguments: '-c $(Build.Configuration) -target:Prepare -p:MaxJdkVersion=$(MaxJdkVersion) -v:diag' - task: DotNetCoreCLI@2 displayName: Shut down existing build server diff --git a/build-tools/automation/templates/core-tests.yaml b/build-tools/automation/templates/core-tests.yaml index 5403c2de1..26e95084f 100644 --- a/build-tools/automation/templates/core-tests.yaml +++ b/build-tools/automation/templates/core-tests.yaml @@ -113,7 +113,7 @@ steps: condition: eq('${{ parameters.runJavaTests }}', 'true') inputs: command: build - arguments: tools/java-source-utils/java-source-utils.csproj /t:RunTests + arguments: tools/java-source-utils/java-source-utils.csproj -t:RunTests -v:diag continueOnError: true - task: PublishTestResults@2 From 856384f8fbbc614236a1303f4fbd0d30fe828317 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Wed, 1 Dec 2021 10:57:50 -0500 Subject: [PATCH 3/6] `java-source-utils` tests need to use the right config. --- build-tools/automation/templates/core-tests.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/build-tools/automation/templates/core-tests.yaml b/build-tools/automation/templates/core-tests.yaml index 26e95084f..a92717f09 100644 --- a/build-tools/automation/templates/core-tests.yaml +++ b/build-tools/automation/templates/core-tests.yaml @@ -113,11 +113,12 @@ steps: condition: eq('${{ parameters.runJavaTests }}', 'true') inputs: command: build - arguments: tools/java-source-utils/java-source-utils.csproj -t:RunTests -v:diag + arguments: -c $(Build.Configuration) tools/java-source-utils/java-source-utils.csproj -t:RunTests -v:diag continueOnError: true - task: PublishTestResults@2 displayName: Publish JUnit Test Results + condition: eq('${{ parameters.runJavaTests }}', 'true') inputs: testResultsFormat: JUnit testResultsFiles: 'tools/java-source-utils/build/test-results/**/TEST-*.xml' From a68ab5e7b4c01801be37cb53330fd5778e378042 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Wed, 1 Dec 2021 11:40:44 -0500 Subject: [PATCH 4/6] Revert "icanhaz diag logs?" This reverts commit e3236936a9ab2cf15910ced2f74e1d13588c970c. We figured out what was going wrong: project configuration! Most of the build was as Release config, meaning e.g. `bin/BuildRelease/JdkInfo.props` was created, but the `java-source-utils` unit tests were run in Debug config, which meant it couldn't find `bin/BuildDebug/JdkInfo.props`, as it didn't exist. This meant that the wrong JDK was used for the tests, thus pain. --- Makefile | 2 +- build-tools/automation/templates/core-build.yaml | 2 +- build-tools/automation/templates/core-tests.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index ecbacacd5..f44a1658f 100644 --- a/Makefile +++ b/Makefile @@ -147,7 +147,7 @@ run-ptests: $(PTESTS) bin/Test$(CONFIGURATION)/$(JAVA_INTEROP_LIB) exit $$r; run-java-source-utils-tests: - $(MSBUILD) $(MSBUILD_FLAGS) tools/java-source-utils/java-source-utils.csproj /t:RunTests -v:diag + $(MSBUILD) $(MSBUILD_FLAGS) tools/java-source-utils/java-source-utils.csproj /t:RunTests bin/Test$(CONFIGURATION)/$(JAVA_INTEROP_LIB): bin/$(CONFIGURATION)/$(JAVA_INTEROP_LIB) cp $< $@ diff --git a/build-tools/automation/templates/core-build.yaml b/build-tools/automation/templates/core-build.yaml index db4a3f5b8..1f47df088 100644 --- a/build-tools/automation/templates/core-build.yaml +++ b/build-tools/automation/templates/core-build.yaml @@ -6,7 +6,7 @@ steps: displayName: Prepare Solution inputs: projects: Java.Interop.sln - arguments: '-c $(Build.Configuration) -target:Prepare -p:MaxJdkVersion=$(MaxJdkVersion) -v:diag' + arguments: '-c $(Build.Configuration) -target:Prepare -p:MaxJdkVersion=$(MaxJdkVersion)' - task: DotNetCoreCLI@2 displayName: Shut down existing build server diff --git a/build-tools/automation/templates/core-tests.yaml b/build-tools/automation/templates/core-tests.yaml index a92717f09..f11aa9014 100644 --- a/build-tools/automation/templates/core-tests.yaml +++ b/build-tools/automation/templates/core-tests.yaml @@ -113,7 +113,7 @@ steps: condition: eq('${{ parameters.runJavaTests }}', 'true') inputs: command: build - arguments: -c $(Build.Configuration) tools/java-source-utils/java-source-utils.csproj -t:RunTests -v:diag + arguments: -c $(Build.Configuration) tools/java-source-utils/java-source-utils.csproj -t:RunTests continueOnError: true - task: PublishTestResults@2 From 62dfcb75ddb3d90a734f4ba3ab1b5bf5efa04d7d Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Wed, 1 Dec 2021 11:49:00 -0500 Subject: [PATCH 5/6] Fix YAML. --- build-tools/automation/templates/core-tests.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build-tools/automation/templates/core-tests.yaml b/build-tools/automation/templates/core-tests.yaml index f11aa9014..9cc8e6990 100644 --- a/build-tools/automation/templates/core-tests.yaml +++ b/build-tools/automation/templates/core-tests.yaml @@ -122,4 +122,4 @@ steps: inputs: testResultsFormat: JUnit testResultsFiles: 'tools/java-source-utils/build/test-results/**/TEST-*.xml' - condition: succeededOrFailed() + continueOnError: true From 715bd452b5fc89f12b29a87e4e32cde8756bb644 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Wed, 1 Dec 2021 12:06:22 -0500 Subject: [PATCH 6/6] Cleanup Remove warning on Windows: ##[warning]No test result files matching **/TEST-*.xml were found. Reformat some `steps` to follow "convention" used elsewhere, e.g. `condition:` after `displayName`. --- build-tools/automation/templates/publish-test-results.yaml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/build-tools/automation/templates/publish-test-results.yaml b/build-tools/automation/templates/publish-test-results.yaml index 6a771acfe..782c367e2 100644 --- a/build-tools/automation/templates/publish-test-results.yaml +++ b/build-tools/automation/templates/publish-test-results.yaml @@ -1,14 +1,16 @@ steps: - task: PublishTestResults@2 displayName: Publish NUnit Test Results + condition: succeededOrFailed() inputs: testResultsFormat: NUnit testResultsFiles: TestResult-*.xml - condition: succeededOrFailed() + continueOnError: true - task: PublishTestResults@2 displayName: Publish JUnit Test Results + condition: ne('$(Agent.OS)', 'Windows') inputs: testResultsFormat: JUnit testResultsFiles: '**/TEST-*.xml' - condition: succeededOrFailed() + continueOnError: true