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

Add matrix build of JDK versions #658

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add matrix build of JDK versions #658

wants to merge 1 commit into from

Conversation

SuperQ
Copy link
Member

@SuperQ SuperQ commented May 2, 2021

Test build on various OpenJDK versions.

Signed-off-by: SuperQ [email protected]

@SuperQ
Copy link
Member Author

SuperQ commented May 2, 2021

Something was broken with the CircleCI webhook. I removed the integration and re-added. Looks like it's building again.

@fstab
Copy link
Member

fstab commented May 2, 2021

FYI: I added TestContainers as part of my OpenTelemetry exemplars branch to test compatibility with different Java versions (including Java 6, which causes the most problems):

@Parameterized.Parameters(name="{0}")
public static String[] images() {
return new String[] {
"openjdk:11-jre",
"openjdk:8-jre",
"ticketfly/java:6",
"ibmjava:8-jre",
};
}

It's not merged yet.

These are two different things: The TestContainers tests make sure that the binary client_java library is compatible with different Java versions, while your change makes sure that you can build the library from source with different versions.

It would be perfectly fine if we require Java 11 to build client_java but release a binary that is compatible with Java 6.

@fstab
Copy link
Member

fstab commented May 2, 2021

TestContainers use Docker to run tests against Docker containers. In order to make them work with CircleCI, we need to switch to machine images:

version: 2
machine: true
jobs:
build:
machine:
image: ubuntu-2004:202010-01
working_directory: ~/circleci-java
steps:
- checkout
- restore_cache:
key: maven-dependencies-{{ checksum "pom.xml" }}
- run: mvn clean verify
- save_cache:
paths:
- ~/.m2
key: maven-dependencies-{{ checksum "pom.xml" }}
orbs:
prometheus: prometheus/[email protected]

Not sure if we can convert your PR to use machine images.

@fstab
Copy link
Member

fstab commented May 2, 2021

I like the idea to have a clear definition which Java versions can be used to build client_java, and I think it's a great idea to have automated tests for that. It would be perfect if we could find a way to do this and still support Docker-based TestContainers.

@SuperQ
Copy link
Member Author

SuperQ commented May 2, 2021

We can run the TestContainers with setup_remote_docker. We can start with any base image like cimg/base.

So basically something like this:

jobs:
  test_containers:
    steps:
    - checkout
    - setup_remote_docker
    - run: docker docker docker

@fstab
Copy link
Member

fstab commented May 2, 2021

I am hoping to get the OpenTelemetry exemplars branch done this week. I'll try the setup_remote_docker then.

Test build on various OpenJDK versions.

Signed-off-by: SuperQ <[email protected]>
@fstab
Copy link
Member

fstab commented Aug 2, 2021

Note: client_java now has a Jetty Jakarta Servlet example (https://github.com/prometheus/client_java/blob/master/simpleclient_servlet_jakarta/src/test/java/io/prometheus/client/exporter/ExampleBenchmark.java) which requires Java 11 to build.

hdost added a commit to hdost/client_java that referenced this pull request Dec 22, 2021
* Also add builds for Java 11 and Java 17

Fixes prometheus#743
Relates prometheus#658

Signed-off-by: Harold Dost <[email protected]>
hdost added a commit to hdost/client_java that referenced this pull request Dec 22, 2021
* Also add builds for Java 11 and Java 17

Fixes prometheus#743
Relates prometheus#658

Signed-off-by: Harold Dost <[email protected]>
hdost added a commit to hdost/client_java that referenced this pull request Dec 22, 2021
* Also add builds for Java 11 and Java 17

Fixes prometheus#743
Relates prometheus#658

Signed-off-by: Harold Dost <[email protected]>
hdost added a commit to hdost/client_java that referenced this pull request Dec 22, 2021
* Also add builds for Java 11 and Java 17

Fixes prometheus#743
Relates prometheus#658

Signed-off-by: Harold Dost <[email protected]>
hdost added a commit to hdost/client_java that referenced this pull request Dec 22, 2021
* Also add builds for Java 11 and Java 17

Fixes prometheus#743
Relates prometheus#658

Signed-off-by: Harold Dost <[email protected]>
hdost added a commit to hdost/client_java that referenced this pull request Dec 22, 2021
* Also add builds for Java 11 and Java 17

Fixes prometheus#743
Relates prometheus#658

Signed-off-by: Harold Dost <[email protected]>
hdost added a commit to hdost/client_java that referenced this pull request Dec 22, 2021
* Also add builds for Java 11 and Java 17

Fixes prometheus#743
Relates prometheus#658

Signed-off-by: Harold Dost <[email protected]>
hdost added a commit to hdost/client_java that referenced this pull request Dec 23, 2021
* Also add builds for Java 11 and Java 17

Fixes prometheus#743
Relates prometheus#658

Signed-off-by: Harold Dost <[email protected]>
hdost added a commit to hdost/client_java that referenced this pull request Dec 23, 2021
* Replaces existing build
** Didn't enable other builds as there are currently failures.

Fixes prometheus#743
Relates prometheus#658

Signed-off-by: Harold Dost <[email protected]>
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