Junit5: Support FilePosition in discovery selectors

[mpkorstanje]

Currently there is no good way to discover and execute a single test when using a file based test system like Cucumber. So I would like to request support for a file position in UriSelector, FileSelector and ClasspathResourceSelector.

Deliverables

Support for a file position when using:

• URI selectors
• File selectors
• Classpath resource selectors

Motivation

Cucumber executes feature files written in Gherkin. A feature file consists of multiple scenarios organized in a hierarchical structure. A project may contain multiple feature files, typically organized in some directory structure.

For example:

|- src/main/java/com/example
| |- App.java
|- src/test/java/com/example
| |- StepDefinitions.java
|- src/test/resource/com/example/
| |- single.feature
| |- rule.feature
| |- outline.feature

Contents of outline.feature:

Feature: A feature with scenario outlines

  Scenario: A scenario
    Given a scenario
    When it is executed
    Then is only runs once

  Scenario Outline: A scenario outline
    Given a scenario outline
    When it is executed
    Then <example> is used

    Examples: With some text
      | example |
      | A       |
      | B       |

    Examples: With some other text
      | example |
      | C       |
      | D       |

outline.feature contains 5 tests in a hierarchy:

A feature with scenario outlines
|- A scenario
|- A scenario outline
| |- With some text
| | |- Example #1
| | |- Example #2
| |- With some other text
| | |- Example #1
| | |- Example #2

Now suppose I would like select both tests (Example #1, Example #2) nested under the example With some text on line 17.

Currently JUnit discovery selectors (ClasspathRootSelector, ClasspathResourceSelector, PackageSelector, FileSelector, DirectorySelector) only allow me to discover all tests in a single directory or file.

The UniqueIdSelector could be used to select the With some text container but requires reproducing the unique id which is rather complicated and quite possibly an implementation detail that should not be relied upon.

Cucumbers currentTestEngine implementation currently allows for a line query parameter to be added in when parsing using the UriSelector. For example:

• classpath:/com/example/outline.feature?line=17
• file:/home/mpkorstanje/projects/cucumber/src/test/resources/com/example/outline.feature?line=17

Since this is not formalized I suspect other test engines that consume the UriSelector may crash on the query parameter and so I would like something safer.

For IDEs being able to use a FileSelector with a file position would also simplify the complexity of providing support for file based test engines. Naively allowing the user to select any line in a file and execute the discovered tests for that line would provide for a minimal implementation without knowing any details about the underlying implementation of the test engine.

The same reasoning applies to ClasspathResourceSelector.

[sbrannen]

Good points!

Note that we actually already have support for FilePosition in FileSource, ClassSource, and ClasspathResourceSource.

See FilePosition.fromQuery(String) for the supported syntax.

Thus, adding the support proposed in this issue would align our FilePosition support in sources and selectors, making that support symmetrical which it arguably should have already been.

I am therefore in favor of adding this support to selectors.

@junit-team/junit-lambda, thoughts?

[sbrannen]

Tentatively slated for 5.7 M1 for _team discussion_

[marcphilipp]

Team decision: Add FilePosition as an optional property of FileSelector and ClasspathResourceSelector.

Challenge: FilePosition is in the ..support.descriptor package which cannot be accessed from the ..discovery package.

[timtebeek]

Hi! First time contributor here; I've started out with adding FilePosition as optional argument to FileSelector and ClasspathResourceSelector here: https://github.com/junit-team/junit5/compare/master...timtebeek:master

Not sure if this was as intended; and it's likely nowhere near done to resolve this issue, but I'd like to get some feedback on what should be the next logical steps. To start with:

• The challenge outline above has me slightly confused as to what would (not) be required to resolve the issue. Can anyone expand on that a bit?
• While @mpkorstanje mentioned he would also like to see file position added to UriSelector there's no mention of it in the team decision; Is that out of scope for this feature?
• Would I need to add methods with FilePosition argument to org.junit.platform.engine.discovery.DiscoverySelectors ?

Any further guidance would be much appreciated; I had thought to open a pull request only if it seems feasible I'll be able to finish the implementation.

[marcphilipp]

@timtebeek Sorry for the delay in response, I hope you're still interested. 🙂

The challenge outline above has me slightly confused as to what would (not) be required to resolve the issue. Can anyone expand on that a bit?

We wouldn't like to duplicate the FilePosition class but cannot reference it from the selectors without introducing a package cycle. Thus, we'll probably have to copy it to the new location, inherit from it in the old location, and deprecate it there. Unless anyone has any better ideas?

* While @mpkorstanje mentioned he would also like to see file position added to `UriSelector` there's no mention of it in the team decision; Is that out of scope for this feature?

UriSource doesn't have it since you can simply add it as query parameters. Thus, we decided it was unnecessary to add to UriSelector. So, yes, it's out of scope for this issue.

Would I need to add methods with FilePosition argument to org.junit.platform.engine.discovery.DiscoverySelectors ?

Yes, since all new constructors should be package-private.

[timtebeek]

@timtebeek Sorry for the delay in response, I hope you're still interested.

No problem at all; happy to help out.

The challenge outline above has me slightly confused as to what would (not) be required to resolve the issue. Can anyone expand on that a bit?

We wouldn't like to duplicate the FilePosition class but cannot reference it from the selectors without introducing a package cycle. Thus, we'll probably have to copy it to the new location, inherit from it in the old location, and deprecate it there. Unless anyone has any better ideas?

Any suggestions as to where to place it? It's my first time contributing here, so not too familiar with where it should go.

* While @mpkorstanje mentioned he would also like to see file position added to `UriSelector` there's no mention of it in the team decision; Is that out of scope for this feature?

UriSource doesn't have it since you can simply add it as query parameters. Thus, we decided it was unnecessary to add to UriSelector. So, yes, it's out of scope for this issue.

Would I need to add methods with FilePosition argument to org.junit.platform.engine.discovery.DiscoverySelectors ?

Yes, since all new constructors should be package-private.

Perfect; will do!

[marcphilipp]

Any suggestions as to where to place it? It's my first time contributing here, so not too familiar with where it should go.

I'm leaning towards adding it to org.junit.platform.engine.discovery so let's start with that.

[timtebeek]

@marcphilipp I've applied your suggestions and opened up https://github.com/junit-team/junit5/pull/2253 for further discussion. :)