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

Ensure All tests run all the time #2842

Merged
merged 1 commit into from
Dec 3, 2022

Conversation

krmahadevan
Copy link
Member

Closes #2841

Fixes #2841 .

Copy link
Member

@juherr juherr left a comment

Choose a reason for hiding this comment

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

Changes are great except the ones on listeners which need more discussions.

import spock.lang.Specification

class SpockSample extends Specification {

@Test
Copy link
Member

Choose a reason for hiding this comment

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

You are not supposed to annotate a spock test.

Copy link
Member Author

Choose a reason for hiding this comment

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

@juherr without the @Test annotation, JUnit refuses to find the method and crashes with No runnable methods error. I have tried this same sample and went back as old as https://mvnrepository.com/artifact/org.testng/testng/6.9.13.6 which was released in Sept 2016 and the behavior is still the same. If you know what is causing this problem, please help point to it. But without this annotation the test is not discoverable by JUnit.

Copy link
Member

Choose a reason for hiding this comment

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

That's the point: a Spock test is not a JUnit test
There is a very specific integration https://github.com/cbeust/testng/blob/563bd6dfe2e74475b45110d1cea8101641ebf2b5/testng-core/src/main/java/org/testng/junit/JUnit4SpockMethod.java and the test was here to check the integration.

If the test is failing, it just means the integration is broken.

Copy link
Member Author

@krmahadevan krmahadevan Dec 2, 2022

Choose a reason for hiding this comment

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

@juherr - This class was created to deal with test results it looks like. I dont see any code which seems to suggest that it was created to discover tests. I say this because this class is getting instantiated ONLY when we are at the point of creating results.

org.testng.junit.JUnit4TestRunner#createTestResult > org.testng.junit.JUnit4TestMethod#JUnit4TestMethod > org.testng.junit.JUnit4TestMethod#getMethod

This is the call stack. I dont see any traces of code that attempts at discovering a Spock test in our codebase.

Copy link
Member Author

Choose a reason for hiding this comment

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

Here's what I see as the exception

image

Copy link
Member Author

Choose a reason for hiding this comment

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

Ignore all this. You were right. I found the problem. It was version upgrades :) We were trying to work with spock 2.x series which expects the JUnit engine like runner to run but TestNG is not going to be there :) so i had to pin us back to the 1.x series of version.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can deprecate the Spock support in the next major release.
In case users have both testng and spock tests, I think the current state of the art is using the junit5 platform with spock and testng flavors.

Copy link
Member

Choose a reason for hiding this comment

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

In fact, same remark for the JUnit support

Copy link
Member Author

Choose a reason for hiding this comment

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

So TestNG 8.0 with the deprecation or we deprecate in 7.7.0 and then get rid of the support for JUnit and spock in 8 ?

@krmahadevan
Copy link
Member Author

@juherr - I have fixed all the comments. Please check now.

@krmahadevan
Copy link
Member Author

@juherr - Now we have a chicken and egg problem

If I upgrade Groovy, then the spock test would fail. If I downgrade Groovy to match the Spock 1.x series, then I see that there are groovy compilation failures in JDK17

See here

What do you suggest we do ? We either rework on the spock support to ensure that it is compliant with the spock 2.x series (or) we disable the Spock Test and move on because its anyways a "deprecation eligible feature"

> Task :testng-core:compileTestGroovy
java.lang.NoClassDefFoundError: Could not initialize class org.codehaus.groovy.vmplugin.v7.Java7
	at org.codehaus.groovy.vmplugin.VMPluginFactory.<clinit>(VMPluginFactory.java:43)
	at org.codehaus.groovy.reflection.GroovyClassValueFactory.<clinit>(GroovyClassValueFactory.java:35)
	at org.codehaus.groovy.reflection.ClassInfo.<clinit>(ClassInfo.java:109)
	at org.codehaus.groovy.reflection.ReflectionCache.getCachedClass(ReflectionCache.java:95)
	at org.codehaus.groovy.reflection.ReflectionCache.<clinit>(ReflectionCache.java:39)
	at org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl.registerMethods(MetaClassRegistryImpl.java:209)
	at org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl.<init>(MetaClassRegistryImpl.java:107)
	at org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl.<init>(MetaClassRegistryImpl.java:85)
	at groovy.lang.GroovySystem.<clinit>(GroovySystem.java:36)
	at org.gradle.api.internal.tasks.compile.ApiGroovyCompiler.parseGroovyVersion(ApiGroovyCompiler.java:315)
	at org.gradle.api.internal.tasks.compile.ApiGroovyCompiler.execute(ApiGroovyCompiler.java:152)
	at org.gradle.api.internal.tasks.compile.ApiGroovyCompiler.execute(ApiGroovyCompiler.java:64)

@krmahadevan
Copy link
Member Author

upgrading Groovy throws this error on the build

> Task :testng-core:compileTestGroovy FAILED
startup failed:
Could not instantiate global transform class org.spockframework.compiler.SpockTransform specified at jar:file:/Users/krishnamahadevan/.gradle/caches/modules-2/files-2.1/org.spockframework/spock-core/1.3-groovy-2.5/6f0df2cf4549dcc4b914971675da7e224e893e82/spock-core-1.3-groovy-2.5.jar!/META-INF/services/org.codehaus.groovy.transform.ASTTransformation  because of exception org.spockframework.util.IncompatibleGroovyVersionException: The Spock compiler plugin cannot execute because Spock 1.3.0-groovy-2.5 is not compatible with Groovy 3.0.10. For more information, see http://versioninfo.spockframework.org
Spock artifact: file:/Users/krishnamahadevan/.gradle/caches/modules-2/files-2.1/org.spockframework/spock-core/1.3-groovy-2.5/6f0df2cf4549dcc4b914971675da7e224e893e82/spock-core-1.3-groovy-2.5.jar
Groovy artifact: file:/Users/krishnamahadevan/.gradle/caches/modules-2/files-2.1/org.codehaus.groovy/groovy/3.0.10/b92c72a758f468e64b55e38abe06afa873decdba/groovy-3.0.10.jar

@juherr
Copy link
Member

juherr commented Dec 3, 2022

I think we have to drop the spock support right now because it is broken since we use jdk17.
Could you remove the related classes in a dedicated pr?

@krmahadevan
Copy link
Member Author

I think we have to drop the spock support right now because it is broken since we use jdk17.
Could you remove the related classes in a dedicated pr?

Sure will do that. Can we go ahead with merging this PR if all is well ?

Copy link
Member

@juherr juherr left a comment

Choose a reason for hiding this comment

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

Please merge once the method is renamed.

@krmahadevan krmahadevan merged commit ca7a3a2 into testng-team:master Dec 3, 2022
@krmahadevan krmahadevan deleted the ensure_all_tests_run branch December 3, 2022 13:39
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.

Not all Unit Tests are running
2 participants