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.

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

Convert Cucumber-Java8 Steps & Hooks to Cucumber-Java #262

Merged
merged 54 commits into from
Sep 30, 2022
Merged

Convert Cucumber-Java8 Steps & Hooks to Cucumber-Java #262

merged 54 commits into from
Sep 30, 2022

Conversation

timtebeek
Copy link
Contributor

@timtebeek timtebeek commented Sep 26, 2022 •
edited
Loading

Converts Steps & Hooks to Cucumber-Java. Fixes #259

There's a few questions marked in the code, as TODO comments;
Any feedback there appreciated, mostly to see if there's a better way.

Git history is quite messy; I suggest a squash if this were to make it in.
If not suitable here, might target cucumber-jvm repository directly.

Tim te Beek added 28 commits September 21, 2022 22:35
Rough outline of CucumberJava8ToCucumberJava
9ae3485
Define RpnCalculator so there's no unresolved references
2b04b4d
Log compilation warnings and errors and add JUnit to classpath
de23f74
Progression
4444976
POC new method
4b46313
Apply once after, instead of per method
f898604
Always call super
9fba390
Adopt visitMethodInvocation now that matching works again
6919dfe
Attempt to split lambda arguments into type safe method arguments
9d61012
Drop attempts to use type safe method parameters for now
35235eb
Individual imports
c70bc31
Rename classes and separate work into distinct visitors
f6e49cd
Minor changes
0dd8404
Verify conversion of methods declared outside constructor
7340dfc
Unpack statements in J.Block
0a2420b
Add autoformat, in vain
38123da
Update formatting and displayName/description
4d71544
Remove hs_err_pid66334.log
df4b579
Revert Java 17 constructs
de8fd26
Remove further Java 17 constructs; add license headers
9af58a4
Add yet another cast after instanceof
fd4a1b5
Rename once again, to better match upcoming hook definition migration
4d55d6f
Consistently use response from super call instead of method argument
b9baba0
Remove TODO items related to missing tests
dcbae6a
Add stub for hook conversion
6cbcd79
Extract a reusable CucumberJava8ClassVisitor
089b14a
First almost working hook conversions
aec424a
Update generated method names
3511fc0
@timtebeek timtebeek changed the title Draft: Convert Cucumber-Java8 to Cucumber-Java Convert Cucumber-Java8 Steps & Hooks to Cucumber-Java Sep 27, 2022
Tim te Beek added 16 commits September 28, 2022 09:27
Cucumber dependencies runtimeOnly
f29b3e3
Add throws Exception where necessary by always adding, then cleanup
ef056f9
Better new method placement
1f83f7e
Add markers when migration is skipped
41f9533
Add a few more after scenario tests after seeing failures in migration
cccf818
Add (failing) test case for Status migration
d54615e
Use ChangePackage for Scenario & Status imports
eeaa7a6
Convert steps before hooks
e4b7df0
Only run cleanup.UnnecessaryThrows when other changes are made
17b5a71
Add test that uses yaml to mimic problems seen in real migrations
0a0a007
Move changePackage to cucumber.yml; Skip matched Scenario.log
15ad4d0
Create a single CucumberJava8ToCucumberJavaTest with @nested sub classes
cd15774
Use fully qualified io.cucumber.java.Scenario in template for now
44277d7
Add spec to anonymous classes test
0d05331
Run RemoveUnneededBlock after migration to simplify body migration
8383f76
Remove added scenario/status imports; package change now retains these
0503e3b
timtebeek
timtebeek commented Sep 29, 2022
View reviewed changes
src/test/java/org/openrewrite/java/testing/cucumber/CucumberJava8ToCucumberJavaTest.java
import static org.openrewrite.java.Assertions.version;

@Issue("https://github.com/openrewrite/rewrite-testing-frameworks/issues/259")
class CucumberJava8ToCucumberJavaTest implements RewriteTest {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Figured create a Java test already, as I heard the Kotlin tests might be going away.

timtebeek
timtebeek commented Sep 30, 2022
View reviewed changes
...in/java/org/openrewrite/java/testing/cucumber/CucumberJava8StepDefinitionToCucumberJava.java
}

private String formatMethodArguments() {
// TODO Type loss here, but my attempts to pass these as J failed: __P__.<java.lang.Object>/*__p0__*/p <error>()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Imports are not an issue in practice, as templates will use the original imports,
but any comments attached to lambda arguments (unlikely) are lost.

timtebeek
timtebeek commented Sep 30, 2022
View reviewed changes
...in/java/org/openrewrite/java/testing/cucumber/CucumberJava8HookDefinitionToCucumberJava.java
private String formatMethodArguments() {
J firstLambdaParameter = lambda.getParameters().getParameters().get(0);
if (firstLambdaParameter instanceof J.VariableDeclarations) {
return String.format("io.cucumber.java.Scenario %s",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fully qualified class name to work around not yet knowing how else to insert the correct type.
Could be either properly fixed here, or through a doAfterVisit to go from FQCN use to import.

@timtebeek
Copy link
Contributor Author

timtebeek commented Sep 30, 2022 •
edited
Loading

CI shows a test failure with TestsShouldNotBePublicTest; that's unrelated and has happened a few times before.

TestsShouldNotBePublicTest > removeProtectedMethodModifiers() FAILED
    java.lang.IllegalStateException: AST contains missing or invalid type information
    MethodDeclaration->Block->ClassDeclaration->CompilationUnit

@jkschneider jkschneider merged commit e02bb26 into openrewrite:main Sep 30, 2022
@timtebeek timtebeek deleted the cucumber-java8 branch September 30, 2022 18:44
@tkvangorder tkvangorder added the recipe Recipe request label Oct 11, 2022
@tkvangorder tkvangorder added this to the 1.29.0 milestone Oct 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
recipe Recipe request
Projects
OpenRewrite
Archived in project
Milestone
1.29.0
Development

Successfully merging this pull request may close these issues.

Migrate from cucumber-java8 to cucumber-java
3 participants
@timtebeek @jkschneider @tkvangorder