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
Add support for spying on concrete objects #77 #695
Conversation
8769bee
to
9d62d65
Compare
9d62d65
to
072d3ea
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @twicksell, thank you for your PR. However, there are some issues:
First, it doesn't work with the Specs own methods
def "can spy on instances"() {
given:
Person person = new Person()
Person spy = Spy(person)
when:
spy.work()
then:
1 * spy.work()
}
This fails with a groovy.lang.MissingMethodException
since you didn't extend org.spockframework.lang.SpecInternals
with the necessary methods.
Furthermore, the Spring xml config should be on par with the DetachedMockFactory
. So you'd need to extend the spock.xsd
, org.spockframework.spring.xml.MockBeanDefinitionParser
, and org.spockframework.spring.xml.SpockMockFactoryBean
.
// configure MockFactory | ||
builder.addConstructorArgValue(element.getAttribute("class")); | ||
builder.addPropertyValue("name", element.getAttribute("id")); | ||
builder.addPropertyValue("mockNature", element.getLocalName()); | ||
if(!element.getAttribute("ref").isEmpty()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it be either ref
or class
, what happens if the class
does not match the actual class of the bean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ref is given priority. I'd love to be able to better specify this in the xsd but can't really. I could be more explicit with startup errors if you prefer and just fail fast, then request the user to specify one or the other but not both.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I think it should fail fast with a helpful error message. As for the xsd, you could add it to the description, unfortunately there is no choice
equivalent in XML Schema for attributes, there seems to be some possibility to achieve this with Schematron or Schemapath, but I don't think it would improve the tool support https://www.w3.org/wiki/Attribute_mutex
8c8ea9f
to
3821244
Compare
3821244
to
a5ebf8a
Compare
… spyRealInstances
Hi guys @leonard84, @twicksell I know this is a quite old PR, but I just noticed that the implementation for Spies on concrete instances does not include the +Closure version of SpyImpl in SpecInternals.java. This means I cannot provide interactions to the Spy, which is usually the purpose of creating them. |
For any following readers, above comment was cross-posted, answer is at #77. |
First attempt at supporting spying of concrete objects.