subjectAction and its discontents

[Joshua Marker]

I’d like to propose a change to the behavior of subjectAction that makes it more useful for behaviorally driven testing. First, I’ll motivate the change by showing a typical BDD pattern that I imagine we’ve all encountered. Then, I’ll make an implementation suggestion. The first doesn’t depend on the second; if the specific suggestion doesn’t fit, another one could be come up with so long as enough people agree with the problem. 

How often have you done this?

(Gist at https://gist.github.com/tooluser/aec912fbc25baee8da6b ) 

```

describe(@“response to a network request or whatnot”)

subjectAction(^{ something that triggers the response})

context(@“When there’s a success of type A”);

context(@“When there’s a success of type B”);

context(@“When there’s a success of type C”);

. . . . 

context(@“when there is an error”)

beforeEach(^{

response = responseThatTriggersError;

});

it(“should display an alert”);

context(“when the user touches ‘ok’”)

beforeEach({

[[UIAlertView currentAlertView] dismissWithOKButton];

});

```

. . . . only to realize now you have to go back, remove the subject action, and put the response-triggering call at the right point in every sub context, because the response that triggers the error is now occurring *after* the touch on the (non-existing) alert? 

SubjectAction is fantastically useful. But it’s fighting with behaviorally testing nested behaviors, and I don’t think it needs to.

Why is this the case? It’s because subjectAction is currently defined to be executed before every example, no matter how deeply nested. This is great for some cases, and rotten for others. In the above case, for example, what one would prefer is for there to be a subjectAction ‘firewall’ at ‘when there is an error’. The subjectAction should be triggered so that the rest of the behavior that proceeds from there - the alert in this case - can be tested. Ideally, you could even start *another* subjectAction within that behavior block!

One can implement this manually, by passing an ‘action’ object and calling it manually. But that’s exactly the overhead that subjectAction is intended to avoid.

Here’s a proposed solution: currently, ‘describe’ and ‘context’ are synonymous. This was done, IIRC (PC, Adam Milligan - maybe Sam or someone else remembers the details?) because both terms are used to about the same effect in two different testing libraries, and this made users of both more at home in Cedar. 

Instead, ‘describe’ should be a subjectAction firewall. That is, SAs get fired by a describe. Thus the above testing could be accomplished by replacing the ‘context(@“when there is an error”)’ line with ‘describe(@“when there is an error”)’.

If there is enough support for this specific solution, I’ll take a hand at implementing it. If there isn’t, but the problem is agreed with, I’m game to hear other suggestions for implementation. 

[Jeffrey Hui]

Hey Joshua,

This is an interesting idea that you proposed. Although I have concerns about the complexity of this feature, for explaining it to users, as well as significantly breaking backwards compatibility. The current behavior of subjectAction is already confusing and difficult to explain. Also, many users do assume that describe and context are synonymous (I also don't have much of the context during the Milligan days for that), which would probably break users that use SA and prefer using describe over context.

Correct me if I'm wrong, but this feature that this seems to propose is to supporting nesting of subjectActions. While it's useful in specific cases, it seems to diverge from the intent of what subjectAction is for - describing the specification of a particular action. It's strict constraint is a plus. There is equivalent feature to SA in Ginkgo, but it is very strongly recommended to only have only JustBeforeEach instead.

There definitely are use cases that subjectAction does not cover well but I'm entirely convinced it should be expanded to support those cases.

Side Note: If you use something like promises, you can get a one-extra context depth since they are order independent:

describe(@"clicking a button", ^{

    beforeEach(^{ stub the http client to return a promise that the test holds });

    subjectAction(^{ click the button });

    it(@"should show a spinner");

    context(@"when the network request succeeds", ^{

        beforeEach(^{ resolve the promise });

        it(@"should display the items in a list");

    });

    context(@"when the network request fails", ^{

        beforeEach(^{ reject the promise });

        it(@"should display an alert");

    });

});

- Jeff

--
You received this message because you are subscribed to the Google Groups "Cedar Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cedar-discus...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Joshua Marker]

This is, in effect, a ‘nesting’ of subject actions. I think that there is a conflict between the SA dogma (‘one per action’) and BDD testing, which inherently (and, I think, to its credit and strength) models multiple actions. 

I see how changing the semantics of describe/context could be confusing to users, though I don’t think it’s more so than changing e.g. the way numeric comparisons coerce. Most users use one or the other. But, that ship has sailed.

I think though that this idea should not be discarded. It bowdlerizes SA, rendering it much less useful for testing behavior. Am I alone in modeling actions that follow other actions, but wanting to use the enormously less verbose SA? 

[Andrew Kitchen]

Hey Joshua,

Agreed that many of us have run into the testing situation that you are describing, but I would suggest the code is asking for a new top-level describe() and possibly a well-named helper block or method.  Also, your subject might be taking on too much responsibility.

I like using subjectAction() for the original behavior you are testing.  I also like it for really test-driving a known set of expected behaviors up front.  Coming in later and adding an adventure test to one of the results is rarely the path to happiness, because we would end up mixing up the results of one test with the setup of a new test we need to write.  BDD-style frameworks let us do this in their simplest usage, but it isn't always good for the test suite or the code under test, and that's precisely where subjectAction() tries to fit in.

subjectAction() is deliberately meant to prevent too much of this "choose your own adventure" style of testing in favor of testing the results of a specific action (the subject of the describe()), given a variety of setup or precursor scenarios.  This doesn't mean it is somehow "un-BDD".  Quite the opposite.  Think of it as enforcing the rhythm of "given/when/then" where the "when" remains constant, allowing us to enumerate more given/then pairs as we discover them.

Circling back, I like the way you've structured your first example.  Here's a copy-paste plus a few minor additions in order to clarify how I think it should read:

describe(@“responding to something important”), ^{

        __block id response;

subjectAction(^{ [subject respond:response]; });

context(@“When there’s a success of type A”, ^{

            beforeEach(@"", ^{

                response = [Responses responseTypeA];

            });

             it(@"should be happy", ^{

                 expect(collaborator).to(be_happy);

             });

        });

context(@“When there’s a success of type B”, ^{

            beforeEach(@"", ^{

                response = [Responses responseTypeB];

            });

             it(@"should be extra happy", ^{

                 expect(collaborator).to(be_happy).with_extra_stuff;

             });

        });

context(@“when there is an error”)

beforeEach(^{

response = [Responses mediocreResponse];

});

it(“should display an alert”, ^{

                    [UIAlertView currentAlertView].title should contain(@"fail");

                });

        });

});

I'm with you this far, more or less.  The "less" is that the alertView test might be misplaced.  Should this be testing a seam? Should we be messaging a delegate here? And anything that happens in response to dismissing the alert, if it is the responsibility of this object, might best represent another describe() -- another given/when/then.

Once you feel the need to start digging in to a "when I dismiss the alert view" I would suggest breaking out a new, top level describe() (same level as our example, or higher) entitled "dismissing the alert view".  For DRY-ness you could consider extracting the subjectAction's block into a local block which you can also call in the beforeEach of this describe().

The reason I'm not yet on board with loosening up the constraints of subjectAction() is that it would allow us to conflate the "when" with the "given" in given/when/then, which is largely what it aims to prevent.  When we think the use of subjectAction() is too strict for the next test we need to add, it means we shouldn't try to add it there, because it would be misplaced.  If you prefer to structure your tests for more exploration, then you probably shouldn't use subjectAction() for those tests.

I hope that's helpful.

-Andrew