Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix or ignore broken JavadocParanamer tests #44

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mjustin
Copy link
Contributor

@mjustin mjustin commented Aug 29, 2021

This fixes JavadocParanamerTest to once again succeed. The tests with a straightforward fix I fixed; the others I (hopefully temporarily) ignored.

I changed the generated Javadoc artifact for the test to not include the version number in the file name. This removes the need to update the version number for each new release of Paranamer.

Note that some of the tests are still brittle in that they are dependent on externally hosted Javadocs. I have not tackled this as part of this PR, though I think improving those so the build doesn't fail if the hosted resources are down or missing in a future PR makes sense.

Newly ignored tests:

  • testLookupParameterNamesForConstructorWithStringArg, testLookupParameterNamesForInterfaceMethod — This was previously working with the 2.5.5 version of the Javadocs, presumably compiled under Java 5. This no longer works as the Javadoc regex no longer appears to match. I tried both the current version using Java 8, and the last released version using Java 7, and neither succeeded. Implementing Support Java 8+ in JavadocParanamer #39 should presumably fix these.
  • dirParanamer, fileParanamer — These were broken by the removal of the Java SE Javadocs from source control, per Unidentified license issue #36. The real fix, IMO, would be to use something without any licensing issue. I think using a generated version of the Paranamer Javadocs instead of the Java SE Javadocs might be the simplest approach.
  • javadocs3, javadocs4 — Oracle no longer hosts these in navigable HTML form, but requires them to be downloaded from their archives. Unsure whether there's an easy way to salvage these directly. I think just hardcoding HTML in Javadoc 3/4 format in the test might be the easiest way to test these going forward (the same approach used in Issue39TestCase. This approach should/could probably (also) be used for newer versions of the Javadocs, so the tests don't break in a similar manner if Oracle moves/removes them, or is temporarily down.

@paul-hammant
Copy link
Owner

I don't think we can do<stripVersion>true</stripVersion> and still publish to maven central. Otherwise, I'm reviewing.

@paul-hammant
Copy link
Owner

What JDK version are you using with Maven?

I'm seeing these test failures:

Failed tests: 
  OldQDoxParanamerTestCase.testGenericClassGeneration:92 expected:<...bo 
elephantArrays E[],java.lang.String th...> but was:<...bo 
elephantArrays E[[]],java.lang.String th...>
  OldQDoxParanamerTestCase.testSimpleClassGeneration:83 expected:<...nger 
longArray long[] longs 
setMap java....> but was:<...nger 
longArray long[[]] longs 
setMap java....>
  QDoxParanamerTestCase.testFoo:76 expected:<...nger 
longArray long[] longs 
setMap java....> but was:<...nger 
longArray long[[]] longs 
setMap java....>

@mjustin
Copy link
Contributor Author

mjustin commented Aug 30, 2021

I don't think we can do<stripVersion>true</stripVersion> and still publish to maven central. Otherwise, I'm reviewing.

What it looks like is happening to me is that this copy is used solely for these unit tests — hence them being copied to the test-data directory used by the tests. The actual Javadocs JAR that's published — which that this is creating a copy of — are untouched. So I don't believe this should affect what's published to Maven Central.

@mjustin
Copy link
Contributor Author

mjustin commented Aug 30, 2021

What JDK version are you using with Maven?

I built this on macOS 10.14.5 with Maven 3.8.2 & Java 8 using AdoptOpenJDK 1.8.0_282.

I'm seeing these test failures:

Failed tests: 
  OldQDoxParanamerTestCase.testGenericClassGeneration:92 expected:<...bo 
elephantArrays E[],java.lang.String th...> but was:<...bo 
elephantArrays E[[]],java.lang.String th...>
  OldQDoxParanamerTestCase.testSimpleClassGeneration:83 expected:<...nger 
longArray long[] longs 
setMap java....> but was:<...nger 
longArray long[[]] longs 
setMap java....>
  QDoxParanamerTestCase.testFoo:76 expected:<...nger 
longArray long[] longs 
setMap java....> but was:<...nger 
longArray long[[]] longs 
setMap java....>

I'm seeing these errors as well, but they are already present in master, and are not added by these changes. I mentioned this in #43 (comment). I think it might have been caused by the recent qdox version increase from 2.0-M3 to 2.0.0.

@mjustin
Copy link
Contributor Author

mjustin commented Aug 30, 2021

Note that these tests have been excluded from the Maven build for since 2010, which is probably a reason these breakages have gone undetected.

paranamer/pom.xml

Lines 80 to 99 in 03c538c

<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>2.19.1</version>
<configuration>
<junitArtifactName>junit:junit</junitArtifactName>
<excludes>
<exclude>**/JavadocParanamerTest.java</exclude>
</excludes>
<forkMode>once</forkMode>
<printSummary>true</printSummary>
<useFile>true</useFile>
<systemPropertyVariables>
<property>
<name>java.awt.headless</name>
<value>true</value>
</property>
</systemPropertyVariables>
</configuration>
</plugin>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants