Ensure All tests run all the time - #2842
Conversation
juherr
left a comment
There was a problem hiding this comment.
Changes are great except the ones on listeners which need more discussions.
|
|
||
| class SpockSample extends Specification { | ||
|
|
||
| @Test |
There was a problem hiding this comment.
You are not supposed to annotate a spock test.
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
In fact, same remark for the JUnit support
There was a problem hiding this comment.
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 ?
7b04d17 to
887632e
Compare
887632e to
1ec284c
Compare
|
@juherr - I have fixed all the comments. Please check now. |
|
@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" |
|
upgrading Groovy throws this error on the build |
|
I think we have to drop the spock support right now because it is broken since we use jdk17. |
Sure will do that. Can we go ahead with merging this PR if all is well ? |
juherr
left a comment
There was a problem hiding this comment.
Please merge once the method is renamed.
1ec284c to
ceccb5a
Compare

Closes #2841
Fixes #2841 .