[CI] Enable Java test in CI workflow by rhdong · Pull Request #805 · rapidsai/cuvs

rhdong · 2025-04-03T22:04:01Z

This PR adds changes for Java CI.

Some scripts modified here also appear in PR #831. Once 831 is merged, I’ll rebase and make sure everything stays consistent.

jameslamb · 2025-04-04T14:49:20Z

@rhdong could you please put this PR into draft until you're ready for reviews? That'd reduce the notifications reviewers are getting, and help them understand when it's time to come review.

rhdong · 2025-04-04T14:58:22Z

@rhdong could you please put this PR into draft until you're ready for reviews? That'd reduce the notifications reviewers are getting, and help them understand when it's time to come review.

Thanks for the reminder! I’ve marked the PR as draft now.

copy-pr-bot · 2025-04-04T15:02:13Z

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

rhdong · 2025-04-04T20:03:47Z

/ok to test

rhdong · 2025-04-07T21:11:41Z

/ok to test

rhdong · 2025-04-07T21:44:41Z

/ok to test

rhdong · 2025-05-05T19:02:47Z

I just tested it locally and it works! Here's a patch for fixing the javadocs plugin issue. We can fix it in a subsequent PR as well, if you think that's better. javadocs-plugin-fix.txt

It fixes these warnings in the build for me:

[WARNING] 
[WARNING] Some problems were encountered while building the effective model for com.nvidia.cuvs:cuvs-java:jar:25.06.0
[WARNING] 'build.plugins.plugin.(groupId:artifactId)' must be unique but found duplicate declaration of plugin org.apache.maven.plugins:maven-javadoc-plugin @ line 257, column 17
[WARNING] 
[WARNING] It is highly recommended to fix these problems because they threaten the stability of your build.
[WARNING] 
[WARNING] For this reason, future Maven versions might no longer support building such malformed projects.
[WARNING]

@chatman thank you! Done the patch!

rhdong · 2025-05-05T19:03:23Z

/ok to test 0f90041

jameslamb

I left some suggestions for your consideration. There are enough of them that I'm leaving a blocking review... please @ me when this is ready for another review.

If you'd prefer that I just push these changes directly to the PR instead, please let me know and I'd be happy to.

Also (not blocking this PR), I think it would be very helpful to reduce the duplication of the Java version across these builds. The assumption of Java 22 is hard-coded in many places right now, and it'd be easy to accidentally miss mentions like this during updates:

https://github.com/rapidsai/cuvs/blob/4a4d86363fd700f487593e65dd9e9ceb321a9c12/java/cuvs-java/pom.xml#L61-L622

If you agree with me about that, that could be a followup PR.

rhdong · 2025-05-06T19:46:15Z

I left some suggestions for your consideration. There are enough of them that I'm leaving a blocking review... please @ me when this is ready for another review.

If you'd prefer that I just push these changes directly to the PR instead, please let me know and I'd be happy to.

Also (not blocking this PR), I think it would be very helpful to reduce the duplication of the Java version across these builds. The assumption of Java 22 is hard-coded in many places right now, and it'd be easy to accidentally miss mentions like this during updates:

https://github.com/rapidsai/cuvs/blob/4a4d86363fd700f487593e65dd9e9ceb321a9c12/java/cuvs-java/pom.xml#L61-L622

If you agree with me about that, that could be a followup PR.

Sure, thank you so much for your suggestions, which are super helpful! For this time, I will submit by myself to avoid duplicate commits, and pls feel free to push directly in next round.

rhdong · 2025-05-06T22:08:29Z

/ok to test 88165ad

rhdong · 2025-05-07T17:22:24Z

I left some suggestions for your consideration. There are enough of them that I'm leaving a blocking review... please @ me when this is ready for another review.

If you'd prefer that I just push these changes directly to the PR instead, please let me know and I'd be happy to.

Also (not blocking this PR), I think it would be very helpful to reduce the duplication of the Java version across these builds. The assumption of Java 22 is hard-coded in many places right now, and it'd be easy to accidentally miss mentions like this during updates:

https://github.com/rapidsai/cuvs/blob/4a4d86363fd700f487593e65dd9e9ceb321a9c12/java/cuvs-java/pom.xml#L61-L622

If you agree with me about that, that could be a followup PR.

Hi @chatman , per James’s comment regarding the Java version, do you have any suggestions or guidance on what the best practice typically is in this case? Thanks!

jameslamb

Thanks for all the changes! I have one more important comment for your consideration. Sorry for not mentioning it earlier.

jameslamb · 2025-05-07T17:45:30Z

+export CMAKE_GENERATOR=Ninja
+


Suggested change

export CMAKE_GENERATOR=Ninja

This can probably be removed here as well, as it was in ci/test_java.sh as part of #805 (comment).

And I see another issue... the change to add libraft and NCCL was not also applied to the test_java group in dependency-file-generator: #805 (comment).

These types of issues are direct result of the high amount of duplication here... I think we should consolidate things.

test_java.sh only differs from build_java.sh in 2 ways:

passes --run-java-tests to java/build.sh

uses the test_java dependency group for rapids-dependency-file-generator (which would be identical to the java group there if libraft and NCCL were correctly added)

I think we should do the following:

remove test_java group in dependencies.yaml

consolidate all of the duplicated stuff in build_java.sh and test_java.sh

for example, by having build_java.sh accept a --run-java-tests argument

that would also ensure build_java.sh is getting tested on PRs (currently it'd only run on nightly builds)

Could I push changes like that to this branch?

I've pushed these changes in 1303422

Let's see how CI goes.

…ava-ci

jameslamb · 2025-05-07T18:17:45Z

/ok to test 1303422

jameslamb · 2025-05-07T18:51:34Z

/ok to test 53f5002

jameslamb

From my perspective, this is ready to merge! The stuff I mentioned about reducing hard-coding of the Java version could come in a later PR (or not at all, if you choose not to pursue it).

Just check the conda-java-tests logs to be sure the tests are running after the most recent fix, otherwise I am ok with merging any time.

rhdong · 2025-05-07T22:21:57Z

/ok to test e69d623

cjnolet · 2025-05-08T14:33:41Z

/merge

…only modify update-version.sh (#875) While reviewing #805, I found an issue with this project's `update-version.sh`... it refers to a script `examples/cmake/thirdparty/fetch_rapids.cmake` which no long exists (removed in #824). This proposes the following: * removing that reference from `update-version.sh` * skipping most CI on PRs that only modify `update-version.sh` ## Notes for Reviewers `ci/release/update-version.sh` is standardized (same filepath, same usage) across almost all RAPIDS repos. I cannot think of a situation where a PR that only changes that file would need to have any CI re-run (other than linting, e.g. for `shellcheck`). If folks agree, I'll roll out a change like that more broadly across RAPIDS. Authors: - James Lamb (https://github.com/jameslamb) - Ben Frederickson (https://github.com/benfred) Approvers: - Ben Frederickson (https://github.com/benfred) - Corey J. Nolet (https://github.com/cjnolet) - Gil Forsyth (https://github.com/gforsyth) URL: #875

[CI] Enable Java test in CI workflow

d81ce11

rhdong requested review from a team as code owners April 3, 2025 22:04

rhdong requested a review from jameslamb April 3, 2025 22:04

github-actions Bot added the ci label Apr 3, 2025

rhdong added feature request New feature or request non-breaking Introduces a non-breaking change labels Apr 3, 2025

rhdong requested a review from cjnolet April 3, 2025 22:04

rhdong mentioned this pull request Apr 3, 2025

[WIP][CI] Enable Java test in CI workflow #795

Closed

rhdong requested a review from raydouglass April 3, 2025 22:07

rhdong added 4 commits April 3, 2025 15:11

confusing but had to change

1363b16

Remove the redundancy test

d23578e

retry with -X when fail

0a11a6e

fix directory for mvn test

ab2610f

rhdong closed this Apr 4, 2025

rhdong reopened this Apr 4, 2025

rhdong marked this pull request as draft April 4, 2025 15:02

add dump file commands

101e166

cjnolet added this to Unstructured Data Processing Apr 7, 2025

cjnolet moved this to In Progress in Unstructured Data Processing Apr 7, 2025

switch to nn-descent

5c8c905

rhdong mentioned this pull request Apr 7, 2025

Update libcuvs libraft ver to 25.06 in conda env #808

Merged

set the signature only under deploy

f46ec06

remove gpg plugin

25579b2

jameslamb requested changes May 6, 2025

View reviewed changes

jameslamb mentioned this pull request May 6, 2025

update.version.sh: remove broken reference, skip most CI on PRs that only modify update-version.sh #875

Merged

Merge branch 'branch-25.06' into rhdong/java-ci

0a88418

fix/optimize script issue per comments

88165ad

rhdong requested a review from jameslamb May 7, 2025 04:19

jameslamb requested changes May 7, 2025

View reviewed changes

jameslamb added 2 commits May 7, 2025 12:59

Merge branch 'branch-25.06' of github.com:rapidsai/cuvs into rhdong/j…

018e1ca

…ava-ci

consolidate build_java.sh and test_java.sh

1303422

fix path

53f5002

raydouglass reviewed May 7, 2025

View reviewed changes

Comment thread ci/release/update-version.sh Outdated

rhdong added 2 commits May 7, 2025 14:20

opt for digits

9ca360f

fix typo

17bb814

jameslamb self-requested a review May 7, 2025 22:19

jameslamb approved these changes May 7, 2025

View reviewed changes

Merge branch 'branch-25.06' into rhdong/java-ci

e69d623

raydouglass approved these changes May 8, 2025

View reviewed changes

cjnolet approved these changes May 8, 2025

View reviewed changes

rapids-bot Bot merged commit d14552a into rapidsai:branch-25.06 May 8, 2025
75 checks passed

github-project-automation Bot moved this from In Progress to Done in Unstructured Data Processing May 8, 2025

cjnolet mentioned this pull request May 27, 2025

[FEA] Java API needs to be built and tested in CI #845

Closed

Conversation

rhdong commented Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jameslamb commented Apr 4, 2025

Uh oh!

rhdong commented Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

copy-pr-bot Bot commented Apr 4, 2025

Uh oh!

rhdong commented Apr 4, 2025

Uh oh!

rhdong commented Apr 7, 2025

Uh oh!

rhdong commented Apr 7, 2025

Uh oh!

rhdong commented May 5, 2025

Uh oh!

rhdong commented May 5, 2025

Uh oh!

jameslamb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rhdong commented May 6, 2025

Uh oh!

rhdong commented May 6, 2025

Uh oh!

rhdong commented May 7, 2025

Uh oh!

jameslamb left a comment

Choose a reason for hiding this comment

Uh oh!

jameslamb May 7, 2025

Choose a reason for hiding this comment

Uh oh!

jameslamb May 7, 2025

Choose a reason for hiding this comment

Uh oh!

jameslamb commented May 7, 2025

Uh oh!

jameslamb commented May 7, 2025

Uh oh!

Uh oh!

jameslamb left a comment

Choose a reason for hiding this comment

Uh oh!

rhdong commented May 7, 2025

Uh oh!

cjnolet commented May 8, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

rhdong commented Apr 3, 2025 •

edited

Loading

rhdong commented Apr 4, 2025 •

edited

Loading