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

[TestNG] Always map Pickles/TestCases to TestNG tests. #1339

Merged
merged 2 commits into from
Apr 1, 2018

Conversation

brasmusson
Copy link
Contributor

Summary

Always map Pickles/TestCases to TestNG tests.

Details

Remove the support for mapping the whole Cucumber test suite or each Cucumber feature to TestNG tests.

Motivation and Context

The first TestNG support implemented in Cucumber-JVM mapped the whole Cucumber test suite to one TestNG test. It did enable running Cucumber from a TestNG runner, but did provide practically no notification of the result in the TestNG domain.

Later the TestNG support was expanded to allow the mapping of each Cucumber feature (file) to a TestNG test. It provided a little better notification of the result in the TestNG domain, but it was still limited.

Now the natural mapping, one Pickle/TestCase to one TestNG test, is supported, and a major release is on the way, the legacy support for mapping the whole Cucumber test suite or each Cucumber feature to TestNG tests should be removed.

How Has This Been Tested?

The automated test suite has been updated to cover the remaining behavior.

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).

Checklist:

  • I've added tests for my code.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

Remove the support for mapping the whole Cucumber test suite or each
Cucumber feature to TestNG tests.
@brasmusson brasmusson added this to the 3.0.0 milestone Mar 31, 2018
@coveralls
Copy link

coveralls commented Mar 31, 2018

Coverage Status

Coverage increased (+0.05%) to 81.581% when pulling cb10e75 on testng-run-pickles-only into f8dd18b on master.

*
* @see AbstractTestNGCucumberTests#runScenario(cucumber.api.testng.PickleEventWrapper, cucumber.api.testng.CucumberFeatureWrapper)
*/
public class PickleEventWrapperImpl implements PickleEventWrapper {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably be package private.

Copy link
Contributor

@mpkorstanje mpkorstanje left a comment

Choose a reason for hiding this comment

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

Do we want to expose TestNGCucumberRunner as part of the public api?

Nevermind. It appears we do. f27890e

Copy link
Contributor

@mpkorstanje mpkorstanje left a comment

Choose a reason for hiding this comment

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

CucumberFeatureWrapper.getCucumberFeature() returns CucumberFeature which falls outside of the public API I think it should be removed from the interface.

It also happens to go unused.

@mpkorstanje
Copy link
Contributor

  • I've tightened up the the modifiers to avoid exposing the internal api.
  • I've removed unused any unused methods.
  • I've removed the javadoc from the *Impl classes. The interfaces should describe everything.
  • I've added a ReadMe that describes how TestNG can be used. Including the composite version I wasn't aware of.

@mpkorstanje
Copy link
Contributor

Everything else looks good to me.

@mpkorstanje mpkorstanje closed this Apr 1, 2018
@mpkorstanje mpkorstanje reopened this Apr 1, 2018
@mpkorstanje mpkorstanje force-pushed the testng-run-pickles-only branch from 566da65 to cb10e75 Compare April 1, 2018 14:26
@mpkorstanje
Copy link
Contributor

I'm moving the read me to a separate PR so I can also include #1338.

@brasmusson brasmusson merged commit cb10e75 into master Apr 1, 2018
brasmusson added a commit that referenced this pull request Apr 1, 2018
@brasmusson brasmusson deleted the testng-run-pickles-only branch April 1, 2018 18:33
@lock
Copy link

lock bot commented Apr 1, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Apr 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants