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

Add support for spock2 #784

Merged
merged 3 commits into from
Dec 20, 2021
Merged

Add support for spock2 #784

merged 3 commits into from
Dec 20, 2021

Conversation

jsalinaspolo
Copy link
Contributor

No description provided.

Signed-off-by: Javier Salinas <jsalinaspolo@gmail.com>
Copy link
Collaborator

@l-1squared l-1squared left a comment

Choose a reason for hiding this comment

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

Thanks a lot for taking the time to do this. I am excited. I'd like to discuss some remarks though before merging.

@@ -74,7 +73,7 @@ subprojects {
jacocoVersion = '0.8.5'
quickcheckVersion = '0.6'
hsqldbVersion = '2.3.2'
byteBuddyVersion = '1.10.2'
byteBuddyVersion = '1.12.3'
Copy link
Collaborator

Choose a reason for hiding this comment

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

good catch :D

@@ -0,0 +1 @@
/target
Copy link
Collaborator

Choose a reason for hiding this comment

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

weird... Isn't there a global gitignore that should exclude that directory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

copy/paste from spock1 module, that was copied from jgiven-junit 😆

Fixing it

new Scenario<GIVEN, WHEN, THEN>(givenClass, whenClass, thenClass)
}

private <T> Class<T> addDoNotInterceptToMetaClass(Class<T> clazz) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I can understand why this exists, but do you happen to know, why it wasn't needed in the spock1 Package?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is related with Groovy 3 vs Groovy 2
The Generated class of MetaClass comes as a method to intercept

}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for polishing :)

import spock.lang.Shared
import spock.lang.Specification

class ScenarioSpec<GIVEN, WHEN, THEN> extends Specification {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe add javadoc and an @since annotation here aswell? After all, the entire module will only be available from 1.2.0 onwards


implementation "org.codehaus.groovy:groovy:$groovy_version"
implementation "org.spockframework:spock-core:$spock2_version"
implementation "org.spockframework:spock-junit4:$spock2_version"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Question: wouldn't it make sense to try to align the spock2 package with JUnit 5 directly? If I recall correctly Spock2 is JUnit5 based and upgrading the published jgiven package later should be significantly harder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Spock2 uses JUnit Platform used in JUnit5, but it does not use JUnit5. Therefore can not use the Junit5 Extensions.

At the end, it is an implementation detail for the user of the library. The user will not see that internally we are using JUnit4 Rules, but we can migrate to native Spock at some point in the future.

Using Spock2 allows users of the library to migrate to JDK17

Signed-off-by: Javier Salinas <jsalinaspolo@gmail.com>
Signed-off-by: Javier Salinas <jsalinaspolo@gmail.com>
@l-1squared l-1squared merged commit 9628b70 into TNG:master Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants