Support for invariants

[xtreme-james-cooper]

A lot of code has some fairly simple invariants - a set of possible UI elements, of which only one can be visible at a time, for instance, or an internal field that can change over the course of use but should never be nil - and it would be nice to quickly verify that none of the use cases in the spec break them, concisely and automatically.

Our quick idea for syntax below:

SPEC_BEGIN(CheckoutViewControllerSpec)

describe(@"CheckoutViewController", ^{

    __block CheckoutViewController* subject;

    invariant(@"only one header label should be shown at at time", ^{
        subject.currentCreditCardLabel.hidden || subject.noCreditCardLabel.hidden should be_truthy;
    });

    beforeEach(^{
        subject = [CheckoutViewController new];
        [subject viewDidLoad];
        [subject viewDidAppear: NO];
     });

    it(@"should not show a pay now button", ^{
        subject.payNowButton.enabled should_not be_truthy;
    });

    context(@"when the user has selected a credit card", ^{
        beforeEach(^{
            [subject creditCardSelected: [[CreditCardModel alloc] initWithNumber:@"1234567890"];
        });

        it(@"should show a pay now button", ^{
            subject.payNowButton.enabled should be_truthy;
        });

        context(@"when the user clicks pay now", ^{
            beforeEach(^{
                [subject.payNowButton sendActionsForControlEvent:UIControlEventTouchUpInside];
            });

            it(@"should show a confirm dialogue", ^{
                subject.confirmDialogue.displayed should be_truthy;
            });

            it(@"should show your redacted card number", ^{
                subject.confirmDialogue.text should equal(@"Do you wish to pay with card xxxxxx7890?");
            });
        });
    });

});

The semantics would be that for every route through the test (as represented by the completion of beforeEach blocks EDIT: probably simpler and clearer just to use contexts and describes) the framework tests all the invariants and treats it as a test failure if any of them is false. In the above example it would test three times: "CheckoutViewController", "CheckoutViewController when the user has selected a credit card" and "CheckoutViewController when the user has selected a credit card when the user clicks pay now" (but only the once, despite the two different tests in the last block).

This would simplify a number of testing patterns and make test suites more robust. (We were inspired by a case similar to the example above, where two labels were superimposed on each other after a complicated flow. We had tests for the flow but only tested the exclusivity of the labels once at the top since it was "clear" that any other case would follow the simple cases. Having invariants would have helped us discover the bug sooner, or at least, having discovered the bug, saved us from an unpleasant surfeit of behavesLike(@"noLabelDuplication"); all over the suite.)

[xtreme-james-cooper]

To be slightly more precise about the semantics, the example above should provide identical results to the following:

SPEC_BEGIN(CheckoutViewControllerSpec)

describe(@"CheckoutViewController", ^{

    __block CheckoutViewController* subject;

    beforeEach(^{
        subject = [CheckoutViewController new];
        [subject viewDidLoad];
        [subject viewDidAppear: NO];
     });

    it(@"INVARIANT - only one header label should be shown at at time", ^{
        subject.currentCreditCardLabel.hidden || subject.noCreditCardLabel.hidden should be_truthy;
    });

    it(@"should not show a pay now button", ^{
        subject.payNowButton.enabled should_not be_truthy;
    });

    context(@"when the user has selected a credit card", ^{
        beforeEach(^{
            [subject creditCardSelected: [[CreditCardModel alloc] initWithNumber:@"1234567890"];
        });

        it(@"INVARIANT - only one header label should be shown at at time", ^{
            subject.currentCreditCardLabel.hidden || subject.noCreditCardLabel.hidden should be_truthy;
        });

        it(@"should show a pay now button", ^{
            subject.payNowButton.enabled should be_truthy;
        });

        context(@"when the user clicks pay now", ^{
            beforeEach(^{
                [subject.payNowButton sendActionsForControlEvent:UIControlEventTouchUpInside];
            });

            it(@"INVARIANT - only one header label should be shown at at time", ^{
                subject.currentCreditCardLabel.hidden || subject.noCreditCardLabel.hidden should be_truthy;
            });

            it(@"should show a confirm dialogue", ^{
                subject.confirmDialogue.displayed should be_truthy;
            });

            it(@"should show your redacted card number", ^{
                subject.confirmDialogue.text should equal(@"Do you wish to pay with card xxxxxx7890?");
            });
        });
    });

});

but much more concisely, and without risk of missing a case or making code-duplication errors.

[akitchen]

Thanks for posting this, @xtreme-james-cooper . Sounds like a useful addition to the toolkit.

I think it's an idea worth exploring - https://www.pivotaltracker.com/story/show/81557010 has been created to track it. A pull request would be welcome as well, if you're actively tinkering with the idea

[idoru]

FWIW I have often written tests which contain assertions in the beforeEach blocks with the implied meaning that assertions not contained in an it block are sanity checks.

Further to this, the effect of testing the invariant for every context combination as demonstrated here could be done in a similar spirit to this using subjectAction.

One thing I do like about @xtreme-james-cooper 's suggestion is that the invariant is elevated to the same level as a test expectation, with explicit prose to call out intent.

[tooluser]

We frequently use the same pattern, and I like it for the same reason.

On Oct 28, 2014, at 12:46 PM, Sam Coward notifications@github.com wrote:

FWIW I have often written tests which contain assertions in the beforeEach blocks with the implied meaning that assertions not contained in an it block are sanity checks.

Further to this, the effect of testing the invariant for every context combination as demonstrated here could be done in a similar spirit to this using subjectAction.

One thing I do like about @xtreme-james-cooper https://github.com/xtreme-james-cooper 's suggestion is that the invariant is elevated to the same level as a test expectation, with explicit prose to call out intent.

—
Reply to this email directly or view it on GitHub #289 (comment).

[xtreme-james-cooper]

Added a pull request for the feature. #294
It turned out to be pretty simple, although I'm sure you all have ideas as to how the code could be improved. There's also a couple of things I decided somewhat arbitrarily and am open to suggestions on:

• Pending tests. After consultation with @pivotal-brian-croom , we decided to make it so that invariants are not run in pending test blocks. This seems reasonable on the one hand since the point of pending tests is to remind you to write them, but on the other hand I can see a use case where we stub out a whole pile of flow and just let the invariant roam automatically over it. it(@"force invariant", ^{true should be_truthy}); at the leaves would work, but obviously that's inelegant.
• Scoping. Invariants seem, to me, to be the kind of thing that you should put at the top of a file en masse and then have true everywhere. However, since they're just created by a function like everything else, you can put them wherever and create context-local invariants. I am not yet sure if this is a feature or a bug.
• Grouping. Related to the above, if you put the invariants into, say describe(@"invariants", ^{...});, the invariants will be treated as local to that block and not run elsewhere or, probably, at all (if the block has no "real" tests in it). Obviously this could be defined away by just explaining that that is a mistake in the wiki, but possibly there should be some way for grouping invariants? Or is just a big list of these at the top of the file fine?
• Other. I look forward to your suggestions.

[xtreme-james-cooper]

@akitchen , @tooluser : thanks!
@idoru : I agree it covers some of the cases for sanity checking in beforeEach/subjectAction blocks, but I think invariants fill a little bit of a different niche. beforeEach tests run, well, before tests, and are useful for testing preconditions: subject.view should_not be_nil; for instance. Invariants run alongside tests, and so should be used for things that are commonly tested but conceptually independent; because they're top-level tests in their own right, they can also do actions. An example might be

invariant(@"should display the appropriate popup on purchase-click", ^{
    [subject.payNowButton sendActionsForControlEvent:UIControlEventTouchUpInside];
    if (! [AlertviewHelper isVCDisplayingAlertView: subject]) {
        if (subject.creditCard != nil) {
           subject.confirmDialogue.displayed should be_truthy;
        } else {
           subject.warningDialogue.displayed should be_truthy;
        }
    }
});

which effectively runs a whole pile of button-spamming tests all throughout your test cases, but without interfering with the tests themselves at all.

(NB: the above isn't very declarative, so I've written an "imply" matcher that makes it read much better:

invariant(@"should display the appropriate popup on purchase-click", ^{
    [subject.payNowButton sendActionsForControlEvent:UIControlEventTouchUpInside];

    BOOL clickPossible = ! [AlertviewHelper isVCDisplayingAlertView: subject];
    (clickPossible && subject.creditCard != nil) should imply subject.confirmDialogue.displayed;
    (clickPossible && subject.creditCard == nil) should imply subject.warningDialogue.displayed;
});

It's not very useful without the invariant code, since the test-writer knows the expected outcome of every test at all times, but it's quite useful with it. I'm not sure whether to move this to its own issue or pull request, because it's basically an offshoot of this issue, but it seemed worth mentioning.)