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

ci: include multiple versions of java and OS in test_pack_dock workflow #1093

Merged
merged 21 commits into from
Apr 20, 2022

Conversation

lionel-nj
Copy link
Contributor

@lionel-nj lionel-nj commented Feb 1, 2022

Summary:

This PR provides support to execute test_pack_doc.yml workflow on multiple versions of Java: 11 and 17, as well as different OS; in order to flag potential incompatibilities.

Expected behavior:

Run the same jobs test, doc, and pack on different versions of the JDK: 11 and 17.

All artifacts should be persisted and identified by the JDK's and OS version that was used to produce them.

Capture d’écran, le 2022-02-01 à 12 53 14

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Run the unit tests with gradle test to make sure you didn't break anything
  • Format the title like "feat: [new feature short description]". Title must follow the Conventional Commit Specification(https://www.conventionalcommits.org/en/v1.0.0/).
  • Linked all relevant issues
  • Include screenshot(s) showing how this pull request works and fixes the issue(s)

@lionel-nj lionel-nj changed the title CI: include multiple versions of java in CI ci: include multiple versions of java in CI Feb 1, 2022
@lionel-nj lionel-nj changed the title ci: include multiple versions of java in CI ci: include multiple versions of java in test_pack_dock workflow Feb 1, 2022
@lionel-nj lionel-nj marked this pull request as ready for review February 1, 2022 14:48
@lionel-nj lionel-nj self-assigned this Feb 1, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Feb 1, 2022

Thank you for this contribution! 🍰✨🦄

Information about source corruption

0 out of 1248 sources are corrupted.

Acceptance test details

The changes in this pull request did not trigger any new errors on known GTFS datasets from the MobilityDatabase.
Download the full acceptance test report for commit 6e04670 here (report will disappear after 90 days).

@lionel-nj
Copy link
Contributor Author

lionel-nj commented Feb 1, 2022

@ed-g I'd like to add to add the OS you're using to the list of OS to be run. You mentioned Linux x86 earlier, could you give information about the distribution that is used on your side please?

@lionel-nj lionel-nj marked this pull request as draft February 1, 2022 17:57
@lionel-nj lionel-nj changed the title ci: include multiple versions of java in test_pack_dock workflow ci: include multiple versions of java and OS in test_pack_dock workflow Feb 1, 2022
@@ -110,11 +116,14 @@ jobs:
- name: Persist cli app jar
uses: actions/upload-artifact@v2
with:
name: Application - cli executable .jar -- ${{ steps.prep.outputs.versionTag }}
name: Application - cli executable - Java ${{ matrix.java_version }}.jar -- ${{ steps.prep.outputs.versionTag }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO this is difficult to read as a name, and it's already hard to find the right Action when looking at "checks" output (see #1095 for a more general discussion of this).

Here's an alternative that I think is easier to read:

Suggested change
name: Application - cli executable - Java ${{ matrix.java_version }}.jar -- ${{ steps.prep.outputs.versionTag }}
name: Application - CLI executable - Java ${{ matrix.java_version }} JAR file -- ${{ steps.prep.outputs.versionTag }}

.github/workflows/test_pack_doc.yml Show resolved Hide resolved
@ed-g
Copy link
Contributor

ed-g commented Feb 2, 2022

@ed-g I'd like to add to add the OS you're using to the list of OS to be run. You mentioned Linux x86 earlier, could you give information about the distribution that is used on your side please?

You bet, it's centos stream 8 (redhat workalike) here's the version from the server;

uname
Linux v 4.18.0-358.el8.x86_64 #1 SMP x86_64 x86_64 GNU/Linux

redhat-release
CentOS Stream release 8

java packages

java-17-openjdk-headless-17.0.1.0.12-2.el8_5.x86_64
java-17-openjdk-17.0.1.0.12-2.el8_5.x86_64
java-17-openjdk-devel-17.0.1.0.12-2.el8_5.x86_64

@ed-g
Copy link
Contributor

ed-g commented Feb 2, 2022

"Ubuntu 20.04" is probably the nearest.

@MobilityData MobilityData deleted a comment from github-actions bot Feb 4, 2022
@MobilityData MobilityData deleted a comment from github-actions bot Feb 4, 2022
@MobilityData MobilityData deleted a comment from github-actions bot Feb 4, 2022
@lionel-nj
Copy link
Contributor Author

lionel-nj commented Feb 4, 2022

It seems that the generation of javadocs on window-latest is problematic due to use of emojis in Strings. I opened #1101 to document this for future work.

As of now, doc job of test_pack_doc workflow is only executed on ubuntu-latest and ubuntu-20.04. After this PR is merge we will potentially be able to replicate the bug regarding unit tests in output-comparator from #1086 (comment).

@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2022

Thank you for this contribution! 🍰✨🦄

Information about source corruption

0 out of 1248 sources are corrupted.

Acceptance test details

The changes in this pull request did not trigger any new errors on known GTFS datasets from the MobilityDatabase.
Download the full acceptance test report for commit 0690b20 here (report will disappear after 90 days).

@barbeau
Copy link
Member

barbeau commented Apr 19, 2022

Hopefully Windows Javadoc generation should work correctly after PR #1121 is merged, which should unblock this PR.

@github-actions
Copy link
Contributor

Thank you for this contribution! 🍰✨🦄

Information about source corruption

0 out of 1248 sources are corrupted.

Acceptance test details

The changes in this pull request did not trigger any new errors on known GTFS datasets from the MobilityDatabase.
Download the full acceptance test report for commit dbb5602 here (report will disappear after 90 days).

@barbeau barbeau marked this pull request as ready for review April 20, 2022 19:34
Copy link
Member

@barbeau barbeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maximearmstrong Do you have any objection to merging this? It seems to be working as intended to me - package/test/javadoc tasks are run across Linux and Windows on GitHub CI now, for both Java 11 and 17.

@maximearmstrong
Copy link
Contributor

@barbeau All good on my side! Thank you!

@maximearmstrong maximearmstrong merged commit 19b0cb3 into master Apr 20, 2022
@maximearmstrong maximearmstrong deleted the ci/multiple-java-versions branch April 20, 2022 20:41
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.

4 participants