Help wanted: review Maven pom.xml changes for test automation

[sideshowbarker]

PR #44 includes a change to the Maven pom.xml file to enable automated testing of the html5lib-tests suite. See 2fbcd75.

This issue is a request for review of that change by anyone with domain expertise/experience with Maven — in particular, with the AntRun plugin.

You’ll notice the pom.xml invokes the AntRun javac and java targets with fork=true. There are comments in the change which explain why, but ideally, it seems preferable to make it work without needing fork=true. So any insights into how that might be possible would be welcome.

@anthonyvdotbe and @carlosame — your reviews would be especially welcome.

[carlosame]

the pom.xml invokes the AntRun javac and java targets with fork=true.

With which JDK version are you running the tests?

[carlosame]

With which JDK version are you running the tests?

Nevermind, I see the [8, 11, 14]. It is unclear what the outcome with 11 and 14 is going to be once the project is modularized, and the fork could make things a bit more 'interesting' in that case.

Is the fork also necessary with JDK 11 and 14?

[sideshowbarker]

Is the fork also necessary with JDK 11 and 14?

I don’t know. It might not be. I can test.

The reason I don’t know yet is that apparently when you have series of jobs to run in GitHub Actions, and the first couple of jobs fail, the remaining jobs get canceled.

So to test whether we’d have the same problem without fork under JDK 11 and 14, maybe I just need to change the config to have [11, 14, 8] (to make the Java 8 jobs run last).

[anthonyvdotbe]

My advice would be to use "the Maven way", i.e. adapt the tests such that the Surefire plugin can run them (it's possible to do so without adopting a test framework, as explained here).

[sideshowbarker]

My advice would be to use "the Maven way", i.e. adapt the tests such that the Surefire plugin can run them

Yeah that would seem to be the better way to do it in the long term — but for the near term it seems good first get something working well enough that we just actually run the tests and see what’s failing, and make any fixes needed to the parser sources to get the tests passing.

(it's possible to do so without adopting a test framework, as explained here)

Thanks — yeah, I personally have zero experience with JUnit (and have never even heard of TestNG…), so it’s encouraging to know we at least wouldn’t have to shoehorn the test harness into one of those just in order to be able to use Surefire.

[anthonyvdotbe]

Please have a look at this branch. It's #43, rebased onto #44, with 4 additional commits:

• move the submodule into place (to allow the tests to find the files without hardcoding paths)
• adjust the POM to no longer skip tests
• add a Html5libTest, which follows the "POJO test" conventions (with minimal changes to other files)
• remove JDK 8 from the test matrix (and use verify instead of package)

So I wouldn't bother with AntRun, since achieving this with Surefire is way easier.

[sideshowbarker]

Please have a look at this branch. It's #43, rebased onto #44, with 4 additional commits:

Thanks much. I tried it just now but I seem to be getting a failure:

``` [INFO] ------------------------------------------------------------------------ [INFO] BUILD FAILURE [INFO] ------------------------------------------------------------------------ [INFO] Total time: 5.582 s [INFO] Finished at: 2020-08-22T22:13:37+09:00 [INFO] ------------------------------------------------------------------------ [ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.22.2:test (default-test) on project htmlparser: There are test failures. [ERROR] [ERROR] Please refer to /Users/mike/workspace/validator/htmlparser/htmlparser/target/surefire-reports for the individual test results. [ERROR] Please refer to dump files (if any exist) [date].dump, [date]-jvmRun[N].dump and [date].dumpstream. [ERROR] There was an error in the forked process [ERROR] null [ERROR] org.apache.maven.surefire.booter.SurefireBooterForkException: There was an error in the forked process [ERROR] null [ERROR] at org.apache.maven.plugin.surefire.booterclient.ForkStarter.fork(ForkStarter.java:656) [ERROR] at org.apache.maven.plugin.surefire.booterclient.ForkStarter.run(ForkStarter.java:282) [ERROR] at org.apache.maven.plugin.surefire.booterclient.ForkStarter.run(ForkStarter.java:245) [ERROR] at org.apache.maven.plugin.surefire.AbstractSurefireMojo.executeProvider(AbstractSurefireMojo.java:1183) [ERROR] at org.apache.maven.plugin.surefire.AbstractSurefireMojo.executeAfterPreconditionsChecked(AbstractSurefireMojo.java:1011) [ERROR] at org.apache.maven.plugin.surefire.AbstractSurefireMojo.execute(AbstractSurefireMojo.java:857) [ERROR] at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:137) [ERROR] at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:210) [ERROR] at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:156) [ERROR] at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:148) [ERROR] at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:117) [ERROR] at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:81) [ERROR] at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build(SingleThreadedBuilder.java:56) [ERROR] at org.apache.maven.lifecycle.internal.LifecycleStarter.execute(LifecycleStarter.java:128) [ERROR] at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:305) [ERROR] at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:192) [ERROR] at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:105) [ERROR] at org.apache.maven.cli.MavenCli.execute(MavenCli.java:957) [ERROR] at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:289) [ERROR] at org.apache.maven.cli.MavenCli.main(MavenCli.java:193) [ERROR] at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) [ERROR] at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) [ERROR] at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) [ERROR] at java.base/java.lang.reflect.Method.invoke(Method.java:564) [ERROR] at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:282) [ERROR] at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:225) [ERROR] at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:406) [ERROR] at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:347) [ERROR] [ERROR] -> [Help 1] [ERROR] [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch. [ERROR] Re-run Maven using the -X switch to enable full debug logging. [ERROR] [ERROR] For more information about the errors and possible solutions, please read the following articles: [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException [ERROR] [ERROR] After correcting the problems, you can resume the build with the command [ERROR] mvn -rf :htmlparser ```

[anthonyvdotbe]

Are you running with JDK 11+? I've been using JDK 14 myself.

[sideshowbarker]

Are you running with JDK 11+? I've been using JDK 14 myself.

$ java --version
openjdk 14.0.1 2020-04-14
OpenJDK Runtime Environment (build 14.0.1+14)
OpenJDK 64-Bit Server VM (build 14.0.1+14, mixed mode, sharing)

[anthonyvdotbe]

My bad, it doesn't work indeed. Not sure what happened there, I'll look into it.

[sideshowbarker]

My bad, it doesn't work indeed. Not sure what happened there, I'll look into it.

OK. Anyway, it looks promising. Would definitely prefer to do it this way if we can make it work.

One thing I notice is this:

[INFO] Running nu.validator.htmlparser.test.DomTest 

I’m not sure if that’s causing the failure, but I think we don’t actually want to be trying to run that class. Instead I think we want to only be running Html5libTest.

[anthonyvdotbe]

It's because the submodule has moved, so you need to update it. After doing so, it works as expected (where "expected" in this case means: the build fails with test failures).

[sideshowbarker]

It's because the submodule has moved, so you need to update it. After doing so, it works as expected (where "expected" in this case means: the build fails with test failures).

Ah, OK — will try that now

[anthonyvdotbe]

One thing I notice is this:

[INFO] Running nu.validator.htmlparser.test.DomTest 

I’m not sure if that’s causing the failure, but I think we don’t actually want to be trying to run that class. Instead I think we want to only be running Html5libTest.

That's because the class name happens to end with "Test". But since it doesn't contain any methods that begin with "test", it doesn't really do anything.

[sideshowbarker]

That's because the class name happens to end with "Test". But since it doesn't contain any methods that begin with "test", it doesn't really do anything.

ah yeah OK

[sideshowbarker]

It's because the submodule has moved, so you need to update it. After doing so, it works as expected (where "expected" in this case means: the build fails with test failures).

OK, did the submodule update and can now confirm that it’s working as expected in my environment.

This is a big improvement — thanks much

I guess now I’ll work on cherry-picking the test changes over from your branch to the #44 branch