Skip to content

Conversation

@jonpryor
Copy link
Contributor

@jonpryor jonpryor commented Nov 30, 2021

Context: 77c9c5f

Commit 77c9c5f 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">
      <javadoc>[<![CDATA[JNI sig: Lcom/xamarin/JavaEnum;]]></javadoc>
…
      </m...> but was:<...um">
      <javadoc>[
        <![CDATA[JNI sig: Lcom/xamarin/JavaEnum;]]>

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 77c9c5f 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.

@jonpryor jonpryor force-pushed the jonp-fix-jsu-regression-77c9 branch 3 times, most recently from e58babe to 871c08c Compare November 30, 2021 19:45
Context: 77c9c5f

Commit 77c9c5f 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">
	      <javadoc>[<![CDATA[JNI sig: Lcom/xamarin/JavaEnum;]]></javadoc>
	…
	      </m...> but was:<...um">
	      <javadoc>[
	        <![CDATA[JNI sig: Lcom/xamarin/JavaEnum;]]>

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 77c9c5f 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.
@jonpryor jonpryor force-pushed the jonp-fix-jsu-regression-77c9 branch 2 times, most recently from 919704d to 8111369 Compare December 1, 2021 02:59
@jonpryor jonpryor force-pushed the jonp-fix-jsu-regression-77c9 branch from 8111369 to e323693 Compare December 1, 2021 03:16
This reverts commit e323693.

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.
@jonpryor
Copy link
Contributor Author

jonpryor commented Dec 1, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines failed to run 1 pipeline(s).

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`.
@jonpryor jonpryor merged commit 2601146 into dotnet:main Dec 1, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Apr 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants