ConformTo matcher added

[AlexDenisov]

Similar matcher to #132.

[NSString class] should conform_to(@protocol(NSCopying));
@"Hello" should_not conform_to("FuuBarBuzz");

[jeffh]

Playing a little devils' advocate here, is checking the existence of a protocol any more useful than testing the method directly? (AKA test what is does and not how it does it). Although if you could present a use case, that's isn't solved by directly testing the method(s) a protocol supports, that works for me.—
Sent from my iPhone

On Sat, Nov 2, 2013 at 5:14 AM, Alexey Denisov notifications@github.com
wrote:

Similar matcher to #132.

[NSString class] should conform_to(@protocol(NSCopying));
[NSString class] should_not conformTo("FuuBarBuzz");

You can merge this Pull Request by running:
git pull https://github.com/railsware/cedar conform_to
Or you can view, comment on it, or merge it online at:
#134
-- Commit Summary --

• ConformTo matcher added
  -- File Changes --
  M Cedar.xcodeproj/project.pbxproj (16)
  A Source/Headers/Matchers/Base/ConformTo.h (31)
  M Source/Headers/Matchers/CedarMatchers.h (1)
  A Source/Matchers/Base/ConformTo.mm (28)
  A Spec/Matchers/Base/ConformToSpec.mm (124)
  -- Patch Links --
  https://github.com/pivotal/cedar/pull/134.patch
  https://github.com/pivotal/cedar/pull/134.diff

[AlexDenisov]

@jeffh, not sure that I've understood you correctly, but here is at least the one case when this matcher will be useful:
https://github.com/railsware/BloodMagic/blob/master/BloodMagic/Specs/Specs/Modules/Lazy/LazySpec.mm#L147

Also I've used both matchers (respond, conform) internally in some projects.

It's not a necessary feature, but just a shorthand.
I don't see any regression, I just tired to include these matchers by myself manually from here

[jeffh]

@AlexDenisov, sorry for the late reply.

I guess I'm arguing that testing protocol conformity goes against the philosophy of TDD in more cases than not. Something along the lines of TDD, where did it all go wrong or Growing Object-Oriented Software, Guided by Tests. These resources basically explain that tests should avoid testing every line of code directly, but the intended behavior. Simply put, don't test the algorithm, but test the result.

So this line in your project: https://github.com/railsware/BloodMagic/blob/master/BloodMagic/Specs/Specs/Modules/Lazy/LazySpec.mm#L151
Covers the same behavior you're testing on line 147 (and 143, for that matter). Although a better assertion for all three of theme would probably be:

describe(@"property with protocol", ^{
    __block BMTestProtocolModel *instance;
    beforeEach(^{
        initializer = [BMInitializer lazyInitializer];
        initializer.protocols = @[@protocol(BMLazyTestProtocol)];
        initializer.initializer = ^id(id sender) {
            instance = [[BMTestProtocolModel new] autorelease];
            return instance;
        };
        [initializer registerInitializer];
    });

    it(@"initializes with based on the protocol", ^{
        subject.propertyWithProtocol should be_same_instance_as(instance);
    });
});

Testing that BMTestProtocolModel supports the protocol isn't behavior that the subject under test (BMLazyModel) doesn't provide. I couldn't find any other examples in that testing protocol conformity in the project your listed there.

I'm just worried that adding a conforms_to matcher doesn't gain anything to testability at the cost of encouraging new testers start testing the incorrect way (using conformsToProtocol: instead of testing what those protocol methods actually do).

If I didn't make it clear or you still disagree please let me know. Granted, if @idoru or @akitchen, find this valuable we can merge it in.

[AlexDenisov]

@jeffh, thanks for the good explanation, your words have sense.

Regarding this concrete case, just want to clarify: actually I care only about that propertyWithProtocol really conform to actual protocol, and not filled with another class instance (as it was in the initial implementation).
Assertions you've mentioned (lines 143, 147, 151) were created in order to cover all cases when this behavior may go wrong. I agree with you, that third assertion covers the expected behavior, but if something goes wrong, then will be much easier to determine what actually broken.

Anyway, thank you for feedback, I've learned a lot, again 😄

[akitchen]

I think there's value in the matcher. It's a bit different from BeInstanceOf, arguably, but not much. At run time vs compile time things are not guaranteed to be what's declared.

I can think of two use cases off the top of my head: testing a provider, and testing the construction of a class at run time. This matcher (like BeInstanceOf) simply moves the check into Cedar's DSL

Of course, Jeff's points about testing behavior are good advice, and worth considering instead of this check in most cases.

On Nov 4, 2013, at 22:23, Jeff Hui notifications@github.com wrote:

@AlexDenisov, sorry for the late reply.

I guess I'm arguing that testing protocol conformity goes against the philosophy of TDD in more cases than not. Something along the lines of TDD, where did it all go wrong or Growing Object-Oriented Software, Guided by Tests. These resources basically explain that tests should avoid testing every line of code directly, but the intended behavior. Simply put, don't test the algorithm, but test the result.

So this line in your project: https://github.com/railsware/BloodMagic/blob/master/BloodMagic/Specs/Specs/Modules/Lazy/LazySpec.mm#L151
Covers the same behavior you're testing on line 147 (and 143, for that matter). Although a better assertion for all three of theme would probably be:

describe(@"property with protocol", ^{
__block BMTestProtocolModel *instance;
beforeEach(^{
initializer = [BMInitializer lazyInitializer];
initializer.protocols = @[@protocol(BMLazyTestProtocol)];
initializer.initializer = ^id(id sender) {
instance = [[BMTestProtocolModel new] autorelease];
return instance;
};
[initializer registerInitializer];
});

it(@"initializes with based on the protocol", ^{
    subject.propertyWithProtocol should be_same_instance_as(instance);
});

});
Testing that BMTestProtocolModel supports the protocol isn't behavior that the subject under test (BMLazyModel) doesn't provide. I couldn't find any other examples in that testing protocol conformity in the project your listed there.

I'm just worried that adding a conforms_to matcher doesn't gain anything to testability at the cost of encouraging new testers start testing the incorrect way (using conformsToProtocol: instead of testing what those protocol methods actually do).

If I didn't make it clear or you still disagree please let me know. Granted, if @idoru or @akitchen, find this valuable we can merge it in.

—
Reply to this email directly or view it on GitHub.

[jeffh]

Fair enough. @AlexDenisov, you could just fix a couple of coding-styles related stuff and I'll merge it in.

[jeffh]

Please make protocol declaration the same line as the class declaration.

[AlexDenisov]

Ok, will do it.

[akitchen]

before merge please:

. use Nil for the class
. use alloc/init instead of new
. move opening curly brace to same line as method signature

[AlexDenisov]

@akitchen, what should I do with constructors/destructor?
Should I apply this changes on RespondTo matcher?

[akitchen]

Let's focus on ConformTo for this pull request; please try to honor the code style using BeInstanceOf.h as an example

https://github.com/pivotal/cedar/blob/master/Source/Headers/Matchers/Base/BeInstanceOf.h

Thank you :)

(oops, edited to indicate ConformTo)

[AlexDenisov]

You mean ConformTo? ;-)

[AlexDenisov]

@akitchen, done.

[jeffh]

Thanks for the contribution @AlexDenisov, sorry for all the troubles I've caused.

[AlexDenisov]

@jeffh, it's okay 👍