Skip to content

Consolidate duplicate CucumberOptions implementations #1773

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.

Sign up for GitHub

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

Jump to bottom
Closed
aslakhellesoy opened this issue Sep 12, 2019 · 4 comments · Fixed by #1774
Closed

Consolidate duplicate CucumberOptions implementations #1773

aslakhellesoy opened this issue Sep 12, 2019 · 4 comments · Fixed by #1774
Labels
⚡ enhancement Request for new functionality
Milestone
5.0.0

Comments

@aslakhellesoy
Copy link
Contributor

Both cucumber-junit and cucumber-testng implement their own CucumberOptions annotation. They are identical and should be moved to cucumber-core.
@aslakhellesoy aslakhellesoy added the ⚡ enhancement Request for new functionality label Sep 12, 2019
@aslakhellesoy
Copy link
Contributor Author

The testng implementation does not have useFileNameCompatibleName and stepNotifications. Perhaps these could be removed, or we could use inheritance?

@mpkorstanje
Copy link
Contributor

I split these intentionally to improve the encapsulation of cucumber-core. The end user should not not have to deal with cucumber-core ever. It does result in some duplication (see also Scenario) but this duplication is worth it IMO. This process is not complete, I still need to work on a solution for Plugins and Events and #1768 will allow us to remove the TypeRegistryConfigurer.

In detail the concrete reasons for introducing runner specific @CucumberOptions were:

  1. There are difference in configuration between TestNG and JUnit and there may be others in the future. Pulling these changes up into core would result in runner specific code paths in core. This was less then ideal.

  2. With the introduction of JPMs we'll have to declare which modules we require and which we require transitively. To keep this simple a typical user of Cucumber with dependencies on

  • cucumber-java
  • cucumber-junit
  • cucumber-pico

Will now only need to import classes from

  • io.cucumber.java
  • io.cucumber.junit
  • io.cucumber.datatable
  • io.cucumber.docstring

This means that cucumber-java can require transitively docstring and datatable so a user who depends on java will automatically have access to everything he needs.

@mpkorstanje
Copy link
Contributor

I still need to work on a solution for Plugins and Events

Comes to mind that it might be worth delaying V5 for this a bit more and pull extract the events and plugin interfaces to a separate module.

@mpkorstanje
Copy link
Contributor

Perhaps these could be removed, or we could use inheritance?

Unfortunately this is not possible with annotations.

@mpkorstanje mpkorstanje added this to the 5.0.0 milestone Sep 12, 2019
@mpkorstanje mpkorstanje mentioned this issue Sep 12, 2019
Merged
3 tasks
mpkorstanje added a commit that referenced this issue Sep 13, 2019
@mpkorstanje
[Core] Extract Plugin module (#1774)
8ee4880 
## Summary

With the introduction of v5 we will break the plugin API by moving
packages from `cucumber.core` to `io.cucumber.core`. We can use this
as an opportunity to extract the plugin and event classes to separate
modules.

This change moves `io.cucumber.core.plugin` to `io.cucumber.plugin` and
`io.cucumber.core.event` to `io.cucumber.plugin.event`. While on its own this
change provides little value it will allow us in the future to provide
better support for plugins.

With the introduction of JPMS we will want to expose these classes for
plugin developers. However we also want to avoid the use of core by most
end users. By moving these classes to their own modules we can avoid the
use of core and future proof the introduction of JPMS.

Closes #1773.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
⚡ enhancement Request for new functionality
Projects
None yet
Milestone
5.0.0
Development

Successfully merging a pull request may close this issue.

[Core] Extract Plugin module cucumber/cucumber-jvm
2 participants
@aslakhellesoy @mpkorstanje