[#325] Spock 2 test support #326

szpak · 2022-08-21T15:21:40Z

And most likely also other 3rd-party JUnit Platform engines.

Also bumped surefire version and added some toString() methods for better reporting in tests.

TODO: All tests pass, but while testing in Idea 2022.1, the Spock test classes are detected as changed, but not executed. I have to bring back how it was possible to debug "live" Infinitest in Idea :). Maybe you will have an idea why?

Fixed #325.

szpak · 2022-08-21T15:35:23Z

infinitest/infinitest-lib/src/main/java/org/infinitest/parser/JavaAssistClass.java

Lines 62 to 71 in 9d5d53e

    
           	isATest = !isAbstract(classReference) && 
        
           			hasTests(classReference) && 
        
           			(isJUnit5TestClass(imports) || canInstantiate(classReference)); 
        
           	className = classReference.getName(); 
        
           } 
        
           private boolean isJUnit5TestClass(final String[] imports) { 
        
           	return stream(imports) 
        
           			.anyMatch(org.junit.jupiter.api.Test.class.getName()::equals); 
        
           }

seems to be a culprit. I wonder how those 2 (3) places (here and DefaultRunner) are sharing responsibility of tests detection?

(a quick a dirty check for Spock tests fixes the execution, but it would be definitely "for Spock only", so we loose the ability to find any junit-platform engine's test - so maybe you have a better idea)

szpak · 2022-08-21T15:43:34Z

infinitest-runner/src/main/java/org/infinitest/testrunner/junit5/Junit5Runner.java

+import java.util.List;
+
+import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
+


I wonder if Junit5Runner shouldn't be renamed to JUnit5PlatformRunner (or something similar) after these changes?

szpak · 2022-08-21T15:49:49Z

After rebased with master, I have the same mockito error, however, I'm not sure yet why it works in master, as I seem to change code in the other places...

gtoison · 2022-08-21T19:31:53Z

Hey @szpak thanks for the contribution and welcome back!
Please bear with me as I'm still trying to wrap my head around Infinitest... actually I would also love to know how to debug Infinitest in Idea (or Eclipse)!
I've just upgraded surefire in #329 and now it seems to report unused mockito stubbings, so I fixed those to. It should be better after rebasing.

gtoison · 2022-08-21T20:00:41Z

infinitest/infinitest-lib/src/main/java/org/infinitest/parser/JavaAssistClass.java

Lines 62 to 71 in 9d5d53e

isATest = !isAbstract(classReference) &&

hasTests(classReference) &&

(isJUnit5TestClass(imports) || canInstantiate(classReference));

className = classReference.getName();

}

private boolean isJUnit5TestClass(final String[] imports) {

return stream(imports)

.anyMatch(org.junit.jupiter.api.Test.class.getName()::equals);

}

seems to be a culprit. I wonder how those 2 (3) places (here and DefaultRunner) are sharing responsibility of tests detection?

(a quick a dirty check for Spock tests fixes the execution, but it would be definitely "for Spock only", so we loose the ability to find any junit-platform engine's test - so maybe you have a better idea)

I think that JavaAssistClass unfortunately needs to replicate the "is it a test" logic of various framework because it discovers tests without actually loading the corresponding classes.
That seems to be a reasonable decision because loading the class within the plugin would create all kind of problems, for instance when the class is recompiled or because it would also need to load all the dependencies.
On the other hand the runner does load the classes (since it will run them) so it can use the JUnit built-in code to check if a class is a test

Looking at the metadata with javaassist "for Spock only" sounds good to me.

szpak · 2022-08-21T20:29:34Z

Keeping infinitest-lib not aware of testing frameworks have sense. Looking at its (our method) usage, it seems to be created to detect test among changed files and pass them to the runner. However, having some future changes around, you could check how it impacts performance (or other thing) to filter it out at the lib level (as it limits the vision to support of junit-platform engines at once :-( ).

Regarding loading the framework related class, it seems the junit-jupiter "was leaked" as a infinitest-lib dependency:

infinitest/infinitest-lib/pom.xml

Lines 69 to 73 in 63d2fc6

    
           <dependency> 
        
           	<groupId>org.junit.jupiter</groupId> 
        
           	<artifactId>junit-jupiter-api</artifactId> 
        
           	<version>${junit5.version}</version> 
        
           </dependency>

and it's effectively loaded:

infinitest/infinitest-lib/src/main/java/org/infinitest/parser/JavaAssistClass.java

Lines 68 to 70 in 63d2fc6

    
           private boolean isJUnit5TestClass(final String[] imports) { 
        
           	return stream(imports) 
        
           			.anyMatch(org.junit.jupiter.api.Test.class.getName()::equals);

Doing the Spock stuff, I can also improve that place to ~~load~~ detect it by name (probably a separate PR to be able to rollback the Spock stuff, just in case ;-) ).

gtoison · 2022-08-22T06:36:42Z

My comments about loading the classes were referring to the user's tests, I think it is fine if JUnit is loaded in the plugin.
At the same time this made me realise that the lib module also gets JUnit through the dependency to the runner and that dependency only seems to be for one exception.
I think it's better to leave these dependencies alone for now, so we don't break too many things at the same time.
The lack of integration tests actually running Infinitest on various sample projects scares me a little.

I'm not sure if you noticed it but I think that the important logic for JUnit tests detection is the one checking if a method is a test method, I think that the check on imports is a last resort.

So I think it's fine to put something specific to Spock here. In case this becomes a problem it shouldn't be too difficult to make it more modular, for instance by checking what test libraries are on the classpath.

szpak · 2022-08-22T21:24:51Z

...test-runner-spock-test/src/test/groovy/org/infinitest/parser/JavaAssistClassSpockSpec.groovy

+
+    void setup() {
+        classPool = new ClassPool()
+        classPool.appendSystemPath()


It's enough in Idea, but with Maven, the tests fails with:

[ERROR] JavaAssistClassSpockSpec.should detect Spock 2 test (#clazz.simpleName):21 Condition failed with Exception: classFor(clazz).isATest() | | | class org.infinitest.testdata.SimpleSpockSpec java.lang.RuntimeException: javassist.NotFoundException: org.infinitest.testdata.SimpleSpockSpec at org.infinitest.parser.JavaAssistClassSpockSpec.classFor(JavaAssistClassSpockSpec.groovy:30) at org.infinitest.parser.JavaAssistClassSpockSpec.should detect Spock 2 test (#clazz.simpleName)(JavaAssistClassSpockSpec.groovy:21) Caused by: javassist.NotFoundException: org.infinitest.testdata.SimpleSpockSpec at javassist.ClassPool.get(ClassPool.java:450) at org.infinitest.parser.JavaAssistClassSpockSpec.classFor(JavaAssistClassSpockSpec.groovy:28) ... 1 more

I extracted it to a separate module to do not have to put Spock 2 on the class path (and possibly any other testing framework which would start to be supported). Maybe you have an idea for the workaround?

szpak · 2022-08-22T21:26:53Z

infinitest-lib/src/main/java/org/infinitest/parser/JavaAssistClass.java

@@ -409,4 +423,37 @@ public boolean locatedInClassFile() {
 	public File getClassFile() {
 		return classFile;
 	}
+
+	private static class SpockIsTestClassChecker {


This class is quite big, so I created an inner class to (at least directly) do not enlarge it. Assuming, there is no way to avoid checking for testing frameworks in the lib module, probably it would be good to extract support for the particular frameworks to separate classes.

And most likely also other 3rd-party JUnit Platform engines. Also bumped surefire version and added some toString() methods for better reporting in tests.

Checking in the lib module.

gtoison · 2022-10-23T12:34:45Z

Hello @szpak, I've made some fixes to get the tests working, and also modified the "is it a test" logic to look for the @Testable annotation instead of Spock's Specification
I've also merged in the recent changes from the master branch

Can you please give me write access to this branch on your repo, so I can push my changes there?

szpak · 2022-10-23T15:49:02Z

Cool, I will take a look!

I gave you a RW access to this branch when I originally made a PR (unless GitHub changed something in that matter recently). Have you tried and failed?

Update. I don't see "Allow edits from maintainers." checkbox in this PR (as mentioned in docs), maybe it is a reason?

szpak · 2022-10-23T15:54:37Z

Hello @szpak, I've made some fixes to get the tests working, and also modified the "is it a test" logic to look for the @testable annotation instead of Spock's Specification

Hmm, interesting. Spock - as far as I know - doesn't use it itself. However, if it works for you, I will take a look (once merged into this branch) to explain why :-).

- Testable is more generic, this might pick up tests from other frameworks as well - Use infinitest-lib utility class to get the tests ClassPool

gtoison · 2022-10-23T20:54:47Z

I'm not sure why I can't push my change (I don't seem to have the permissions) so I've submitted here: szpak-forks#1
I've reverted most of the changes in JavaAssistClass so compared to the master the change is only looking for @Testable in the class or its parents

szpak · 2022-10-23T23:24:11Z

Cool, I merged your change manually with fast-forward and after a quick look, @Testable is from the platform, it might be possible to detect tests recognized by the JUnit Platform.

Regarding write permissions, I created that form many years ago. Probably that feature was not available right there. I added you to the repository directly, if you want to add anything else.

szpak · 2022-10-23T23:25:23Z

infinitest-lib/src/main/java/org/infinitest/parser/JavaAssistClass.java

 		className = classReference.getName();
 	}

-	private boolean isJUnit5TestClass(final String[] imports) {
+	private boolean hasJUnit5TestImport(final String[] imports) {
 		return stream(imports)
 				.anyMatch(org.junit.jupiter.api.Test.class.getName()::equals);


It's outside a scope of this PR, but probably @Testable could be used also for JUnit Jupiter tests.

functional-tests/infinitest-runner-spock-test/pom.xml

szpak · 2022-10-23T23:33:28Z

I will play with it more tomorrow, but in general it looks good.

gtoison · 2022-10-24T06:53:11Z

Thank you! You seem to be the author of the ClassPoolForFakeClassesTestUtil that fixed the issues with the ClassPool, but that was a while ago :)
From what I've seen the problem was that JavaAssist could not load the Specification class at all, so it could check if that was the parent class (or check if it had the @Testable annotation).
This is a bit specific to Infinitest's own tests, when actually running it should get the classpath of the project under test and the Spock classes should be available

szpak · 2022-10-24T21:12:56Z

Thank you! You seem to be the author of the ClassPoolForFakeClassesTestUtil that fixed the issues with the ClassPool, but that was a while ago :)

Adding support for TestNG in 2013... I could forgot about that :-).

Regarding @Testable, it is definitely used in spock.lang.Specification. Previously, I did just a quick check in GitHub and its seach field hadn't returned any result (possibly I made a typo or something...).

added java unit tests for to check the logic

gtoison · 2022-10-26T19:18:36Z

Thanks a lot for the contribution!

szpak mentioned this pull request Aug 21, 2022

Support for Spock 2 tests (junit-platform based) #325

Closed

szpak force-pushed the spock2Infinitest branch 2 times, most recently from f48f91c to f9f1976 Compare August 21, 2022 15:25

szpak force-pushed the spock2Infinitest branch from f9f1976 to 42ae973 Compare August 21, 2022 15:39

szpak commented Aug 21, 2022

View reviewed changes

szpak force-pushed the spock2Infinitest branch from 42ae973 to a5f6031 Compare August 21, 2022 19:34

szpak commented Aug 22, 2022

View reviewed changes

szpak force-pushed the spock2Infinitest branch from cb740ac to 6c7a20e Compare August 22, 2022 21:34

szpak added 2 commits August 22, 2022 23:35

[infinitest#325] Spock 2 tests support

dd8295a

And most likely also other 3rd-party JUnit Platform engines. Also bumped surefire version and added some toString() methods for better reporting in tests.

[infinitest#325] Spock 2 tests support

b818af8

Checking in the lib module.

szpak force-pushed the spock2Infinitest branch from 6c7a20e to b818af8 Compare August 22, 2022 21:35

gtoison added the comp:core core feature of infinitest (infinitest-lib + infinitest-runner)) label Aug 24, 2022

szpak mentioned this pull request Sep 28, 2022

Error gettting module working directory for multi-module project in Idea #344

Closed

szpak marked this pull request as ready for review October 23, 2022 15:49

szpak marked this pull request as draft October 23, 2022 15:49

fix: look for @testable instead of Spock's Specification

a271634

- Testable is more generic, this might pick up tests from other frameworks as well - Use infinitest-lib utility class to get the tests ClassPool

szpak commented Oct 23, 2022

View reviewed changes

functional-tests/infinitest-runner-spock-test/pom.xml Show resolved Hide resolved

szpak marked this pull request as ready for review October 24, 2022 21:12

szpak and others added 2 commits October 24, 2022 23:15

[infinitest#325] Upgrade Spock to 2.3.

bde75da

fix: use an early return in isJUnit5Testable() for clarity

65c259c

added java unit tests for to check the logic

gtoison force-pushed the spock2Infinitest branch from f9e62c1 to 65c259c Compare October 26, 2022 19:09

gtoison merged commit 1dbc697 into infinitest:master Oct 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#325] Spock 2 test support #326

[#325] Spock 2 test support #326

szpak commented Aug 21, 2022 •

edited

szpak commented Aug 21, 2022 •

edited

szpak Aug 21, 2022 •

edited

szpak commented Aug 21, 2022

gtoison commented Aug 21, 2022

gtoison commented Aug 21, 2022

szpak commented Aug 21, 2022 •

edited

gtoison commented Aug 22, 2022

szpak Aug 22, 2022

szpak Aug 22, 2022

gtoison commented Oct 23, 2022

szpak commented Oct 23, 2022 •

edited

szpak commented Oct 23, 2022

gtoison commented Oct 23, 2022

szpak commented Oct 23, 2022 •

edited

szpak Oct 23, 2022

szpak commented Oct 23, 2022

gtoison commented Oct 24, 2022

szpak commented Oct 24, 2022

gtoison commented Oct 26, 2022

		import java.util.List;

		import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;

[#325] Spock 2 test support #326

[#325] Spock 2 test support #326

Conversation

szpak commented Aug 21, 2022 • edited

szpak commented Aug 21, 2022 • edited

szpak Aug 21, 2022 • edited

Choose a reason for hiding this comment

szpak commented Aug 21, 2022

gtoison commented Aug 21, 2022

gtoison commented Aug 21, 2022

szpak commented Aug 21, 2022 • edited

gtoison commented Aug 22, 2022

szpak Aug 22, 2022

Choose a reason for hiding this comment

szpak Aug 22, 2022

Choose a reason for hiding this comment

gtoison commented Oct 23, 2022

szpak commented Oct 23, 2022 • edited

szpak commented Oct 23, 2022

gtoison commented Oct 23, 2022

szpak commented Oct 23, 2022 • edited

szpak Oct 23, 2022

Choose a reason for hiding this comment

szpak commented Oct 23, 2022

gtoison commented Oct 24, 2022

szpak commented Oct 24, 2022

gtoison commented Oct 26, 2022

szpak commented Aug 21, 2022 •

edited

szpak commented Aug 21, 2022 •

edited

szpak Aug 21, 2022 •

edited

szpak commented Aug 21, 2022 •

edited

szpak commented Oct 23, 2022 •

edited

szpak commented Oct 23, 2022 •

edited