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

Exception using new consumer version selector method in kotlin #1594

Closed
stefluhh opened this issue Aug 9, 2022 · 15 comments
Closed

Exception using new consumer version selector method in kotlin #1594

stefluhh opened this issue Aug 9, 2022 · 15 comments

Comments

@stefluhh
Copy link
Contributor

stefluhh commented Aug 9, 2022

Hi,

I started to migrate the @Deprecated consumerVersionSelectors = [ ] in the @PactBroker annotation by the new way, using @PactBrokerConsumerVersionSelectors method like this. See bottom line:

image

However when running my PactTest that ran using commented-out code above, I get this exception:

image

I debugged a bit your library and figured, that this line tries to invoke the identified selectorsMethod with a testInstance param that is Optional.empty() though. I find it strange that invokeSelectorsMethod thinks my selector method has 1 param even though it doesn't. This circumstance is explicitly handled in testClassHasSelectorsMethod:

image

  • Kotlin version: 1.7.10
  • Pact version: 4.3.13
  • junit5: yes, PactVerificationInvocationContextProvider

What else I tried:

  • Moving my selector method out of abstract class and into to concrete test class
  • Making it "kotlin static" leads to not running into my breakpoint in testClassHasSelectorsMethod even, because its not contained in testClass?.kotlin?.members:
companion object {
        @PactBrokerConsumerVersionSelectors
        @JvmStatic
        fun consumerVersionSelectors(): SelectorBuilder = SelectorBuilder().branch("my-branch")
    }
@stefluhh stefluhh changed the title Exception using new consumer version selector method Exception using new consumer version selector method in kotlin Aug 9, 2022
@rholshausen
Copy link
Contributor

The documentation is wrong, the method needs to be public to be invoked.

@rholshausen
Copy link
Contributor

Ah! You created a PR to fix the docs. What a champion!

@rholshausen
Copy link
Contributor

rholshausen commented Aug 10, 2022

I find it strange that invokeSelectorsMethod thinks my selector method has 1 param even though it doesn't

Instance methods have one extra parameter, which is the this pointer.

How is your actual test extending the abstract one?

@stefluhh
Copy link
Contributor Author

stefluhh commented Aug 10, 2022

Thanks for support! It looks quite unspectacular to me:
image

@rholshausen
Copy link
Contributor

I need to investigate the JUnit 5 docs, because it looks like it is not providing a test class instance for your test, and I need the instance to be able to call that method on it.

@stefluhh
Copy link
Contributor Author

I did a bit more experimentation about this and added @TestInstance(TestInstance.Lifecycle.PER_CLASS) to my test class. I get a testInstance then which is of type of my test class itself (ProductPactProviderTest), but the Exception remains the same:

java.lang.IllegalArgumentException: object is not an instance of declaring class

image

@stefluhh
Copy link
Contributor Author

stefluhh commented Aug 10, 2022

It does work if I unwrap the instance out of the Optional, however I don't know the jUnit5 extension specifics enough in order to understand why the testInstance is missing. In fact, it is already missing on very beginning in PactVerificationInvocationContextProvider. Here the param context.testInstances is null already.

Edit: BTW I meanwhile replaced @ExtendWith(PactInvocationVerificationContextProvider::class) by SpringRunner instead, since my Provider spins up a Spring context, but this wasn't the problem either.

@rholshausen
Copy link
Contributor

For Spring tests, it is better to use @ExtendWith(PactVerificationSpringProvider::class)

@rholshausen
Copy link
Contributor

Looks like this is the old chicken and egg problem. A test instance is created for each interaction to verify, and we need the selectors to fetch the Pacts so we can create the test instances. So when the selector method is called, there will never be a test instance.

This is why it needs to be static. The other option is to create an new instance to call with, which will work as long as the selector method does not try to use any test class instance variables. They will not be set.

@rholshausen
Copy link
Contributor

I've updated the function, it will support both normal functions, abstract base classes as well as companion objects now. See https://github.com/pact-foundation/pact-jvm/blob/master/provider/junit5/src/test/kotlin/au/com/dius/pact/provider/junit5/ConsumerVersionSelectorKotlinTest.kt

@stefluhh
Copy link
Contributor Author

Awesome, thanks.

@viktorgt
Copy link

I stumbled over the same issue today, since we also migrating from tags to branches/environments. When can we expect a new release?

@stefluhh
Copy link
Contributor Author

@viktorgt PACT has released the fix tonight.

@stefluhh
Copy link
Contributor Author

Fix confirmed, this code works now:

@PactBrokerConsumerVersionSelectors
fun consumerVersionSelectors(): SelectorBuilder = SelectorBuilder()
     .branch("main")
     .environment("prod")
     .environment("qa")
     .environment("int")

Thanks a lot for this fast workflow, fix released just 3 days later, that's amazing.

@mefellows
Copy link
Member

Thanks for raising! It takes many hands :)

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

No branches or pull requests

4 participants