Improve CI upstream tests

abelsromero commented 1 year ago

Kind of change

[ ] Bug fix
[ ] New non-breaking feature
[ ] New breaking feature
[ ] Documentation update
[x] Build improvement

Description

What is the goal of this pull request?

Improve CI job that validates the current code is compatible with upstream Asciidoctor.

How does it achieve that?

Bumps tools when possible (newer versions of gem-maven-plugin don't work correctly) and some cleanup of the script and poms to remove unnecessary parts.

Are there any alternative ways to implement this?

The goal is to install the gem in a maven compatible way so that com.github.jruby-gradle.base can resolve it. I natively tried manually overriding the local gem files in asciidoctorj/asciidoctorj-core/build/.gems/gems but then the gradle jruby plugin fails because it cannot still resolve it. Also reviewed com.github.jruby-gradle docs to see if we can do the installation, but there's no option.

We could install via native Ruby gem command, but that means installing Ruby in the pipeline and I personally prefer it without it to keep things simpler and cleaner.

Are there any implications of this pull request? Anything a user must know?

Yes, always keep your CI healthy :green_circle:

Issue

If this PR fixes an open issue, please add a line of the form:

Fixes #1178

For branch 2.5.x the fix will be the same but without bumping versions because newer ones are not Java 8 compatible.

Release notes

Please add a corresponding entry to the file CHANGELOG.adoc

Not worth it, is just some cleanup and minor improvement. A quick look at the history I think, the issue was only in v2.5.x branch and I was side tracked by other issues related to certain combinations with gem-maven-plugin and the Maven.

abelsromero commented 1 year ago

Seems there's some real test failure, I'll have a look

> Task :asciidoctorj:pollutedTest

org.asciidoctor.WhenAnAsciidoctorClassIsInstantiatedInAnEnvironmentWithGemPath > should_have_gempath_in_ruby_env_when_created_with_gempath FAILED
    org.jruby.exceptions.LoadError at org/jruby/RubyKernel.java:1057

org.asciidoctor.WhenAnAsciidoctorClassIsInstantiatedInAnEnvironmentWithGemPath > should_have_gempath_in_ruby_env_when_created_with_default_create FAILED
    org.jruby.exceptions.LoadError at org/jruby/RubyKernel.java:1057

org.asciidoctor.WhenAnAsciidoctorClassIsInstantiatedInAnEnvironmentWithGemPath > should_not_have_gempath_in_ruby_env_when_created_with_null_gempath FAILED
    org.jruby.exceptions.LoadError at org/jruby/RubyKernel.java:1057

mojavelinux commented 1 year ago

This may have been due to changes I was making last night to upgrade the Asciidoctor core build to 9.4.2.0. It should be working now.

abelsromero commented 1 year ago

This may have been due to changes I was making last night to upgrade the Asciidoctor core build to 9.4.2.0. It should be working now.

Not so sure, I am seeing some odd things:

In my local the fix works fine with J11 and J17, but the original code fails.
In PR pipelines it fails with the error (with the fix).
In PR pipelines it fails, the original code also fails.
Scheduled build on main branch has been working without changes (original code). :point_left: that's the crazy part

abelsromero commented 1 year ago

Quick update: I got it working (main, PRs and local) and still don't know why the latest version of gem-maven-plugin (2.0.1) does not work in CI, but does locally. But I wouldn't discard some silly mistake, it's late.

I will update this PR with these changes https://github.com/asciidoctor/asciidoctorj/compare/main...abelsromero:asciidoctorj:issue-1178-step-by-step-test?expand=1. In short, not many changes, I was sidetracked by the errors in v2.5.x with where related to the repository configuration as pointed by @robertpanzer https://github.com/asciidoctor/asciidoctorj/pull/1180#issuecomment-1510406748.

The only interesting bit is using s4u/setup-maven-action@v1.7.0 to setup also the maven version and make sure we control that variable of the pipeline, the actions/setup-java uses older maven 3.8.8 instead of latests 3.9.1. That's maintained by Slawomir Jaranowski and simplifies java and maven setup, hard to find any issues imho. We could use mvn wrapper, but I don't like adding several files to the repo when we can get the same with a simple declarative yaml.

abelsromero commented 1 year ago

:green_circle:

robertpanzer commented 1 year ago

Thanks for the hard work!

asciidoctor / asciidoctorj