create_jvm_test_suite: add optional test_name_prefix option

[dmivankov]

This should allow having multiple test suites within same package that share test sources.

Can be useful to run same test code in different configurations (env vars, deps or other parameters)

[gemini-code-assist]

Summary of Changes

Hello @dmivankov, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a new optional parameter to the create_jvm_test_suite function, which allows for a custom prefix to be added to generated test names. This enhancement addresses the need for defining multiple test suites that can utilize the same underlying test source files within a single package, thereby supporting diverse testing configurations.

Highlights

• New test_name_prefix parameter: An optional test_name_prefix parameter has been added to the create_jvm_test_suite function. This parameter defaults to an empty string.
• Enhanced Test Naming: The test_name_prefix is now prepended to the generated test names within the create_jvm_test_suite function, allowing for unique test names even when sharing source files.
• Support for Multiple Test Suites: This change enables the creation of multiple test suites within the same package that can share common test sources, facilitating testing under various configurations without naming conflicts.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces a test_name_prefix option to create_jvm_test_suite, which is a useful feature for avoiding test name collisions when defining multiple suites from the same sources. The implementation is straightforward. I've found one potential issue with how test names are generated when source files are located in subdirectories, which could lead to invalid target names. I've left a specific comment with a suggestion to make the name generation more robust. Otherwise, the changes look good.

[gemini-code-assist]

The current implementation for test_name can lead to invalid Bazel target names. The src variable can contain slashes if source files are in subdirectories (e.g., from a glob like **/*.java). A target name with a slash is invalid and will cause a build failure.

To fix this, you should sanitize the generated test name by replacing slashes with another character, like an underscore. This will also make the rule more robust against a test_name_prefix that might contain slashes.

Suggested change

	test_name = test_name_prefix + src[:suffix]
	test_name = (test_name_prefix + src[:suffix]).replace("/", "_")

[jeffmace]

Adding test_name_prefix as an argument sets a precedent where additional arguments may be added in the future. Alternatively, you could achieve this with a custom define_test method. The create_jvm_test_suite macro populates the test_suite with the name returned by define_test. The implementation would look something like this.

def custom_test_suite(name, test_name_prefix, **kwargs):
  def _define_test(test_name, **test_kwargs):
    test_name = test_name_prefix + test_name

    java_test(
      test_name,
      **test_kwargs
    )
    return test_name

  create_jvm_test_suite(
    name = name,
    define_test = _define_test,
    **kwargs
  )

https://github.com/dmivankov/rules_jvm/blob/f8220a5374e9f88ca93dfe6b45efd1f494f7392a/java/private/create_jvm_test_suite.bzl#L130