Skip to content

Commit

Permalink
Remove Sputnik
Browse files Browse the repository at this point in the history
  • Loading branch information
leonard84 committed Oct 31, 2019
1 parent c4c6920 commit fa8bd57
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 153 deletions.
108 changes: 0 additions & 108 deletions spock-core/src/main/java/org/spockframework/runtime/Sputnik.java

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -13,53 +13,8 @@
*/
package org.spockframework.runtime

import org.junit.runner.manipulation.Filter
import org.junit.runner.Description
import org.junit.runner.manipulation.Sorter

import spock.lang.Specification

class SputnikSpec extends Specification {
def "description reflects subsequent filtering"() {
def sputnik = new Sputnik(SputnikSampleSpec)

expect:
sputnik.description.children.methodName == ["feature1", "feature2", "feature3"]

when:
sputnik.filter(new Filter() {
@Override
boolean shouldRun(Description description) {
description.methodName == "feature2"
}
@Override
String describe() {
"filter"
}
})

then:
sputnik.description.children.methodName == ["feature2"]
}

def "description reflects subsequent sorting"() {
def sputnik = new Sputnik(SputnikSampleSpec)

expect:
sputnik.description.children.methodName == ["feature1", "feature2", "feature3"]

when:
sputnik.sort(new Sorter(new Comparator<Description>() {
int compare(Description desc1, Description desc2) {
desc2.methodName <=> desc1.methodName
}
}))

then:
sputnik.description.children.methodName == ["feature3", "feature2", "feature1"]
}
}

class SputnikSampleSpec extends Specification {
def feature1() {
expect: true
Expand Down

29 comments on commit fa8bd57

@harshahegde
Copy link

Choose a reason for hiding this comment

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

Hello @leonard84 what is the alternate for Sputnik.class if i am using latest spockframework jar ?

@Vampire
Copy link
Member

Choose a reason for hiding this comment

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

@harshahegde
Sputnik is a JUnit 4 runner
Spock 2.0 will be based on JUnit 5 though, so you don't need a separate JUnit 4 runner anymore

@harshahegde
Copy link

Choose a reason for hiding this comment

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

Hello @Vampire does that mean I do not need @PowerMockRunnerDelegate() annotation if I want to combine spock with junit?
When i try to run junit with spock and when i am not using this annotation I get exceptions like this

java.lang.Exception: No runnable methods

at org.powermock.modules.junit4.internal.impl.testcaseworkaround.PowerMockJUnit4MethodValidator.validateInstanceMethods(PowerMockJUnit4MethodValidator.java:77)
at org.junit.internal.runners.MethodValidator.validateMethodsForDefaultRunner(MethodValidator.java:51)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.validate(PowerMockJUnit44RunnerDelegateImpl.java:124)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.<init>(PowerMockJUnit44RunnerDelegateImpl.java:86)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl.<init>(PowerMockJUnit47RunnerDelegateImpl.java:42)
at org.powermock.modules.junit4.internal.impl.PowerMockJUnit49RunnerDelegateImpl.<init>(PowerMockJUnit49RunnerDelegateImpl.java:25)
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
at org.powermock.modules.junit4.common.internal.impl.JUnit4TestSuiteChunkerImpl.createDelegatorFromClassloader(JUnit4TestSuiteChunkerImpl.java:165)
at org.powermock.modules.junit4.common.internal.impl.JUnit4TestSuiteChunkerImpl.createDelegatorFromClassloader(JUnit4TestSuiteChunkerImpl.java:47)
at org.powermock.tests.utils.impl.AbstractTestSuiteChunkerImpl.createTestDelegators(AbstractTestSuiteChunkerImpl.java:107)
at org.powermock.modules.junit4.common.internal.impl.JUnit4TestSuiteChunkerImpl.<init>(JUnit4TestSuiteChunkerImpl.java:69)
at org.powermock.modules.junit4.common.internal.impl.AbstractCommonPowerMockRunner.<init>(AbstractCommonPowerMockRunner.java:36)
at org.powermock.modules.junit4.PowerMockRunner.<init>(PowerMockRunner.java:34)

@harshahegde
Copy link

Choose a reason for hiding this comment

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

@leonard84 @Vampire
I am using Junit 4 . If I have to migrate from spockframework version "1.3-groovy-2.5" to "2.0-M1-groovy-2.5" then do I have to upgrade Junit version to 5.x as well?

@Vampire
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 what I just said already.
Spock 2.0 is JUnit 5 based, not JUnit 4 anymore, what was misunderstandable with that?
If you use PowerMockRunnerDelegate, you use the PowerMockRunner, which is JUnit 4 and thus is - as said - not compatible with Spock 2.0 which is JUnit 5 based.
Also no, you don't need PowerMockRunnerDelegate to make Spock 1.x work with JUnit, you need Sputnik which is annotated in the base Specification class.
If you overwrite this though with the PowerMockRunner, you need the delegate annotation to make PowerMock work with Spock (partly actually).

So to repeat myself again, yes you need JUnit 5 to use Spock 2.
If you have JUnit 4 tests, those can also be run with JUnit 5 using the Vintage engine.
JUnit 5 tests run on the Jupiter engine, Spock tests run on the Spock engine.

Also, PowerMock is not yet compatible with JUnit 5.
And even if it were compatible with JUnit 5, it would probably only be compatible with the Jupiter engine and not with the Spock engine I guess.

It might maybe work (as far as it actually works already) if you use the spock-junit4 module which supports JUnit rules in the Spock engine and use one of the PowerMock rules to bootstrap PowerMock instead of the PowerMockRunner.

@kriegaex
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry to ask here instead of creating a new issue - please advise me to do so if this is not the right place - but for testing e.g. static methods in existing Java classes which cannot be refactored because they are external dependencies (such as application launchers like JavaFX launcher or similar) I cannot use (normal or global) GroovyMocks, i.e. I need a way to run Powermockito with Spock 2. How can I do that if there is no compatibility layer? A sample project (POM + Java class + Spock 2 test using Powermock) would be helpful. Thank you. This is a blocker for people with existing tests if they want to upgrade to Spock 2.

@Vampire
Copy link
Member

Choose a reason for hiding this comment

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

Uhm, which part of my last message was unclear?

@kriegaex
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand this, at least not enough to configure my project in a way to be able to try it, which is why I asked for a sample project.

It might maybe work (as far as it actually works already) if you use the spock-junit4 module which supports JUnit rules in the Spock engine and use one of the PowerMock rules to bootstrap PowerMock instead of the PowerMockRunner.

Actually I did try, but maybe I did not do the right thing because it did not work. I know that is a fuzzy error description, but I would like to start with something an expert like you thinks looks most promising.

@Vampire
Copy link
Member

Choose a reason for hiding this comment

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

I have no sample project and I don't know whether it is promising, I actually doubt it.
If you don't tell or show what you did, noone can find a suggestion without a crystal ball and mine is broken unfortunately.
Without more info I cannot say much more than what I already said. Add the spock-junit4 module to your dependencies and bootstrap PowerMock with one of their rules as they have documented it, then try whether it works.

@leonard84
Copy link
Member Author

Choose a reason for hiding this comment

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

Unless someone writes a PowerMockExtension for Spock, I don't see a way to use PowerMock with Spock 2.0.
On the upside, once that is done, it will be better integrated into Spock.
There is also an open issue #735 to get this functionality directly into Spock without requiring PowerMock.

@Vampire
Copy link
Member

Choose a reason for hiding this comment

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

You don't think the PowerMock rule can work @leonard84?
(I didn't look into its implementation)

@kriegaex
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @leonard84 and @Vampire. By the way, #735 does not address mocking/stubbing static methods, it is mainly about final classes/methods. Static mocking would still require PowerMock or GroovyMocks (which don't work for Java code).

@kriegaex
Copy link
Contributor

@kriegaex kriegaex commented on fa8bd57 Mar 21, 2020

Choose a reason for hiding this comment

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

With old versions of Spock (1.0), Groovy (2.4.x) and PowerMock (1.6.6) I could use @Rule PowerMockRule ... in this sample project (tag powermock-rule). But when trying to upgrade to Spock 1.3, Groovy 2.5.8 and PowerMock 2.0.2 and Mockito 2 API (just switch to master, I added two commits today), I could not use it anymore, seeing the same errors as in Spock 2.0 with Groovy 3. To me it seems as if this stopped working a long time ago already, but I am not sure due to which component. The sample project compiles and runs for both variants. Maybe you want to take it from there. Just try to run PersonTest. There is also an EasyMock test variant in the latest commit. The symptoms are the same with EasyMock and Mockito when using Powermock.

P.S.: This is why I switched to the runner instead of the rule a while ago in other projects, but now this bites me due to the missing Sputnik runner in Spock 2.

@Vampire
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for your example project, that gave me something to quickly play with.
I did so and played with your project and PowerMock.
I got it running fine with Spock 1.3 and even 2.0.
It is like I assumed, while maybe having a proper Spock extension will provide better integration, using the agent rule works perfectly fine for 1.3 and also for 2.0 with the spock-junit4 module for rules support.

With "seeing the same errors" I guess you refer to "Missing stackmap frame" verify errors?

Let me shortly explain what is the problem, maybe you are interested and @leonard84 too.
If not, just skip this and know that you either have to wait for a fix in PowerMock (PR: powermock/powermock#1043)
or use the work-around mentioned in there.

The non-agent PowerMock rule was never compatible with Spock, as it checks for methods with the JUnit 4 @Test annotation or for JUnit 3 test methods and Spock feature methods do fulfill neither criterion.

The agent PowerMock rule worked fine with Spock 1.0 and 1.1 and ceased to work with Spock 1.2 and newer.
Actually you can get the same error with 1.0 and 1.1 depending on the other libs you use, but not for Spock classes.
The relevant difference between 1.1 and 1.2 (that could pretty well also exist in any other lib you use) is the class file format.
Up to 1.1 Java 6 was supported, which did not know Stackmap Frames, so there were none in the class files and there were also none expected during class verification.
1.2 dropped support for Java 6, so the class files were compiled with a newer class file format which included Stackmap Frames which were added in Java 7.
So that is the trigger of the bug in PowerMock.

Unfortunately the Agent-based class manipulation uses an unfortunate combination of flags for ByteBuddy which causes it to throw away the Stackmap Frames.
This then is complained about in the verifier.

My PR I linked above fixes that issue, so that it does not happen and as work-around the class verification can be disabled with the JVM flag -noverify.

At least the tests in your test project run fine with both solutions.

@kriegaex
Copy link
Contributor

@kriegaex kriegaex commented on fa8bd57 Mar 22, 2020

Choose a reason for hiding this comment

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

Thanks @Vampire for investigating this. Actually I saw two types of errors, the VerifyError with the agent and a NPE with the non-agent rule.

I was actually referring to the latter, not knowing that it never worked due to missing @Test annotations anyway. I just saw that the test runner could not identify and test methods because in class PowerMockRule it tries to create a PowerMockStatement like this:

return new PowerMockStatement(base, testSuiteChunker.getTestChunk(method.getMethod()), mockPolicyInitializer);

But getTestChunk(...) returns null which leads to the NPE down in the PowerMockStatement constructor.

As for your -noverify workaround, it actually works when using the agent rule, thank you very much. I do hope that your PR will be accepted and released quickly for PowerMock and that there will also be Spock maintenance releases 1.x.x and 2.x to upgrade to that new version. For now I am thinking about compiling my own PowerMock version with your fix and use that instead of adding -noverify to all my tests manually or globally in the IDE and in build scripts.

Edit: Hm, I can browse your latest force-pushed (uh-oh) commit directly in the PowerMock repo. Are you a committer there? It does not seem to be a PR. But strangely, I cannot see the corresponding branch on GithHub, as if the branch as such was not pushed and your commit was a detached head. Maybe I am lacking Git knowledge there, but can you maybe explain that?

@Vampire
Copy link
Member

Choose a reason for hiding this comment

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

The branch is in my fork. In the upstream repo the commit is also on a branch, but in a ref-spec that is not fetched by default.

There will be no 1.x release of Spock. Also what for, Spock has no dependency on PowerMock and there is no change necessary.

@kriegaex
Copy link
Contributor

@kriegaex kriegaex commented on fa8bd57 Mar 22, 2020

Choose a reason for hiding this comment

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

Sorry, I had just noticed the commit was in your fork and wanted to delete my edit, but you answered at the same time. Sorry to trouble you. As for Spock, of course there is no PowerMock dependency. I am juggling with so many balls (tools, dependencies) right now, I was not thinking logically for a moment.

Update: I (as a Maven user) just figured out how to skip tests in a Gradle build and installed your fix in my local Maven cache as version PowerMock 2.0.6-pr_1043. It is working and I do not need -noverify anymore. Great job, I am really happy. Now I will upgrade from 1.3 to 2.0-M2 and see if it still works, but I absolutely expect it to.

@kriegaex
Copy link
Contributor

Choose a reason for hiding this comment

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

Another update: In this branch of my Spock PowerMock playground project I have upgraded to Spock 2, Groovy 3, JUnit 5 and @Rule PowerMockRule using JUnit 4 compatibility layers in both Spock and JUnit. The only prerequisite is a local install build of your (@Vampire) bugfix with version number 2.0.6-pr_1043 as referenced to by my own POM.

Feel free to comment on my POM. This was my first encounter ever with JUnit 5 and Spock 2, so maybe something is suboptimal, even though working fine (tested with Java 8, I did not upgrade the Java version).

@leonard84
Copy link
Member Author

Choose a reason for hiding this comment

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

Great that you both worked it out.

@Vampire
Copy link
Member

Choose a reason for hiding this comment

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

Now we are definitely out of the scope of a commit comment.
If you have further questions maybe better join the Spock Slack.

Feel free to comment on my POM.

Well, as you explicitly asked for my opinion, my first comment would be not to use Maven, but Gradle. :-D
But from a more substantive point of view, there are maaaany most probably unneeded dependencies there.
You shouldn't blindly include every module you find but see what they do. :-D

  • there is no need to include the maven-surefire-plugin explicitly unless you really want to use that milestone version, but for using Spock it is not necessary
  • Unless you want to also write non-Spock tests, there is no need for any of the JUnit dependencies, but I'll mentionen them individually too
  • You do not need junit-jupiter-engine, unless you want to write plain JUnit 5 tests alongside the Spock tests. Spock ships with its own JUnit 5 engine
  • You do not need junit-vintage-engine, unless you want to write old JUnit 4 tests, this is mainly for migrating a project where the tests are still in JUnit 4 and get successively migrated to JUnit 5
  • You do not need junit-platform-engine, unless you want to write an own JUnit 5 engine
  • You do not need junit-platform-testkit, unless you want to test a JUnit 5 engine or extension
  • Spock no longer depends on groovy-all, but only on groovy (in 2.0-M2 on groovy and some other groovy-xxx modules, from 2.0-M3 on only on groovy and you need to add any additional modules you need in your tests yourself)
  • powermock-classloading-xxx are for the non-agent rule, for the agent-based rule you need neither of them

@kriegaex
Copy link
Contributor

Choose a reason for hiding this comment

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

Now we are definitely out of the scope of a commit comment.

Yes, I am aware of that. It was not my plan either to have a long discussion here. But we had some positive results: You fixed an old PowerMock bug, I managed to set up a Maven project using Spock 2 with Groovy 3 and PowerMock + Mockito/EasyMock.

If you have further questions maybe better join the Spock Slack.

I have never used Slack, but if it has a web front-end and you tell me how to find the Spock channel, I will be glad to join. On the Spock homepage I didn't find any information about it.

Feel free to comment on my POM.

Well, as you explicitly asked for my opinion, my first comment (...)

I want to reply to you there, but will refrain from doing so for the time being and take it to Slack instead, as you suggested. Just so much for now: Several of the dependencies you said are unnecessary are indeed necessary.

@Vampire
Copy link
Member

@Vampire Vampire commented on fa8bd57 Mar 23, 2020

Choose a reason for hiding this comment

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

@kriegaex
Copy link
Contributor

@kriegaex kriegaex commented on fa8bd57 Mar 23, 2020

Choose a reason for hiding this comment

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

Du hast noch keinen Account in diesem Workspace?
Bitte den Workspace-Administrator um eine Einladung.

Oh, BTW, isn't Slack a commercial product? Can I just create an account for free? If no, is there another way to get in touch in an interactive way?

Refer to my StackOverflow profile for ways to contact me and ask for my e-mail address.

@szpak
Copy link
Member

@szpak szpak commented on fa8bd57 Mar 23, 2020

Choose a reason for hiding this comment

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

@Vampire I believe Slack was initially created as a place for the internal discussion of Spock contributors.

@kriegaex For general discussions https://gitter.im/spockframework/spock might be better.

@Vampire
Copy link
Member

Choose a reason for hiding this comment

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

Oh, sorry, confused that.
Slack is free, yes.
And you can use it freely with some restrictions.
There are commercial plans though.

But as Marcin said, better join Gitter then and ask further questions there.

@Vampire
Copy link
Member

@Vampire Vampire commented on fa8bd57 Apr 19, 2020

Choose a reason for hiding this comment

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

@leonard84 & @kriegaex
I just stumbled upon another way to use PowerMock with Spock 2.0 and without the limitations of the agent-based JUnit rule and without the need for the Spock JUnit 4 compatibility layer.
Believe it or not, but it indeed is the PowerMockRunner.
It is a bit from behind through the chest in the eye, but it works.

JUnit 5 has a JUnit 4 runner called JUnitPlatform, that lets you run any JUnit platform based test in a JUnit 4 environment, so that you can use JUnit platform based tests in environments (IDEs, build tools, CI servers, ...) that do not yet naturally support JUnit platform.

Leveraging this, this works:

  • Annotate your Spock 2.0 based specification with @RunWith(PowerMockRunner) to run through the PowerMock runner
  • Annotate your Spock 2.0 based specification with @PowerMockRunnerDelegate(JUnitPlatform) to instruct the PowerMock runner to delegate to the runner that runs the JUnit platform based test
  • Make sure your test executor executes tests with JUnit 4

I tested this with Gradle and the test cases from the Spock PowerMock playground project and it works perfectly fine.
If there is some auto-detection like "If it is @Testable run with JUnit 5 else, ..." and no way to select the JUnit version manually, you might be at loss.
But then you always have the choice to switch to Gradle where the choice is explicit. :-D

@kriegaex
Copy link
Contributor

@kriegaex kriegaex commented on fa8bd57 Apr 21, 2020

Choose a reason for hiding this comment

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

Thanks a lot, @Vampire. I know that this is "just" a commit comment discussion, but it is so valuable, it should be published elsewhere. ;-)

I gave it a quick spin and it works from Maven after adding these dependencies to my existing project:

    <version.junit-platform>1.5.2</version.junit-platform>

    <!-- 2.0.7 contains an important bug fix making '@Rule PowerMockRule' usable from Spock -->
    <version.powermock>2.0.7</version.powermock>

    <!-- (...) -->

    <!-- Needed for: PowerMockRunner, PowerMockRunnerDelegate -->
    <dependency>
      <groupId>org.powermock</groupId>
      <artifactId>powermock-module-junit4</artifactId>
      <version>${version.powermock}</version>
      <scope>test</scope>
    </dependency>

    <!-- Needed for: JUnitPlatform -->
    <dependency>
      <groupId>org.junit.platform</groupId>
      <artifactId>junit-platform-runner</artifactId>
      <version>${version.junit-platform}</version>
      <scope>test</scope>
    </dependency>

In IntelliJ IDEA some (not all) PowerMock tests are failing, though. Maybe there something is wrong with the test runner, I will have to investigate and update my comment later.


Update: Now suddenly the same tests are failing in Maven too. Maybe a previous clean did not work well, I had a shell open in a target subdirectory and just noticed. @Vampire, maybe we can sort it out on Gitter if I don't find out by myself until you wake up in your time zone. A list of your test-related dependencies would be good, maybe I have some I should actually delete now that I changed the test running strategy.


Update 2: Okay, I got the tests running in Maven again after explicitly adding this dependency inside the Surefire plugin configuration:

          <dependencies>
            <dependency>
              <groupId>org.apache.maven.surefire</groupId>
              <artifactId>surefire-junit47</artifactId>
              <version>2.22.2</version>
            </dependency>
          </dependencies>

It still does not affect IntelliJ IDEA. This PowerMock setup for Spock seems to create more problems than it solves. My previous setup at least was working for Spock 2 and JUnit 5. For now I have reverted my project to @Rule PowerMockRule powerMockRule = new PowerMockRule() and the Spock JUnit 4 compatibility layer.

@szpak
Copy link
Member

@szpak szpak commented on fa8bd57 Apr 21, 2020

Choose a reason for hiding this comment

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

@kriegaex Write a blog post about it with sample Gradle/Maven configuration and you will (probably) help a lot of people :-). However, it would be even better to mention it somewhere in the PowerMock documentation - it's most likely the first port of call for people looking for help with PowerMock.

Btw, generic information about it, will be also included in Spock documentation.

@Vampire
Copy link
Member

Choose a reason for hiding this comment

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

Yes, adding the surefire-junit47 dependency is what is necessary for Maven to fulfill the Make sure your test executor executes tests with JUnit 4 part, otherwise it recognizes that it is a JUnit 5 test and uses a 5-runner just like IntelliJ does (https://youtrack.jetbrains.com/issue/IDEA-238167)

In Gradle it is simply removing (or not adding) the useJUnitPlatform() line as there no auto-detection happens right now but you always select manually which test runner you want.

And as IntelliJ can delegate test-execution to Gradle in a nicely integrated way (which is also the default when you import a Gradle project), that is also a way to make it work from within IntelliJ. :-)

Please sign in to comment.