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

Part I - Introduce ModuleUtils with ClassFinder SPI #1061

Closed
wants to merge 2 commits into from
Closed

Conversation

sormuras
Copy link
Member

@sormuras sormuras commented Sep 13, 2017

Overview

This PR prepares the support for selecting Java 9 modules for test discovery.

Basic idea: publish a String-based service providing interface for naming module(s) of interest and let an explicit module register a service implementation for actually finding classes in the module graph at runtime. This service implementation should be hosted in a different (sub-) project, like junit-platform-commons-jpms. See #1057

At runtime, using Java 9, the user can add the org.junit.platform.commons.jpms module to the module-path and specify which module he wants to test. The mods directory contains the JUnit 5 artifacts (loaded as automatic modules), the org.junit.platform.commons.jpms module, the application modules (tested objects), and the test modules (classes with @Test annotated methods):

java
--module-path
  mods
--add-modules
  ALL-MODULE-PATH
--module
  org.junit.platform.console
--select-module
  jpms.integration

New Console Launcher Options

  • --scan-module-path
    Scan all modules on the runtime boot module-path for test discovery.
    See http://mail.openjdk.java.net/pipermail/jigsaw-dev/2017-September/013180.html for details.

  • -o or --select-module with single argument <module name>
    Select single module for test discovery. This option can be repeated.
    Same runtime boot module-path as above, but only the test classes of the named modules are selected.

TODO


I hereby agree to the terms of the JUnit Contributor License Agreement.


Definition of Done

@sormuras sormuras added this to the 5.1 Backlog milestone Sep 13, 2017
@sormuras sormuras self-assigned this Sep 13, 2017
@sbrannen sbrannen mentioned this pull request Sep 13, 2017
8 tasks
@sormuras sormuras changed the title Part I - Add module selection support Part I - Introduce ModuleUtils with ClassFinder SPI Sep 13, 2017
@sormuras sormuras force-pushed the module branch 8 times, most recently from f1e959d to eb40607 Compare September 16, 2017 22:59
/**
* Return list of classes found in the named module that may contain testable methods.
*
* @param moduleName name of the module to inspect or {@code ALL-MODULE-PATH}
Copy link
Member

Choose a reason for hiding this comment

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

I think having two separate methods, e.g. findAllClassesInModule() and findAllClassesInModulePath(), would be a cleaner. Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

Having two methods makes sense here, because we have two different selectors in the launcher part. The blue-print where I took the special module name "ALL-MODULE-PATH" from only has a single command line option, namely: --add-modules. There is no --add-all-module-path or in our world: --scan-module-path.

tl;dr: Two separate methods are good - and remove the special module name "ALL-MODULE-PATH".

"EXPERIMENTAL: Select single module for test discovery. This option can be repeated.") //
.withRequiredArg() //
.describedAs("module name");

Copy link
Member

Choose a reason for hiding this comment

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

Should we also add support in the Vintage engine?

Copy link
Member Author

Choose a reason for hiding this comment

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

Mh? Vintage should support both new selectors. If it doesn't then something was lost in rebasing to master.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, missed that! LGTM!

@sormuras sormuras modified the milestones: 5.1 Backlog, 5.1 M1 Sep 18, 2017
List<Class<?>> classes = new ArrayList<>();

logger.config(() -> "Loading auto-detected class finders...");
int serviceCounter = 0;
Copy link
Member Author

Choose a reason for hiding this comment

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

We could limit the single service implementation to be our own ... by comparing the class name or some other "magic ID". Or is it okay to allow third-party implementations here? Thoughts?

This commit includes the service providing interface `ModuleClassFinder`
in package `org.junit.platform.commons.util`. Its default implementation
will rely on Java 9 features introduced by the Java Platform Module
System. The usage of the new module selection options on Java 8 will
result in no-op as there is no module system in place.

Summary of changes:

 * Add ModuleSelector to the Platform discovery package.
 * Add ModuleSelector to the Jupiter discovery package.
 * Add ModuleSelector to the Vintage discovery package.
 * Add ModuleUtils to the platform commons utilities toolbox.
 * Add option `--select-module` to the console launcher.
 * Add option `--scan-module-path` to the console launcher.

Addresses #425
@sormuras
Copy link
Member Author

sormuras commented Oct 7, 2017

Superseded by #1087

@sormuras sormuras closed this Oct 7, 2017
@ghost ghost removed the status: in progress label Oct 7, 2017
@sormuras sormuras deleted the module branch October 17, 2017 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants