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

Create an id<TyphoonComponentFactoryMutator> that can insert mocks for some definitions #58

Closed
jasperblues opened this issue Aug 31, 2013 · 11 comments

Comments

@jasperblues
Copy link
Member

An id can be attached to the factory to mutate definitions.

So far we have the TyphoonPropertyPlaceholderConfigurer. .

I'm thinking it might be nice to attach a mutator that will tell the assembly to override a given component definition, replacing it with a mock or stub.

This would be useful when you want to do integration testing (test a class along with its collaborators), but configuring a certain part of the assembly to behave in accordance with the test scenario.

@rhgills
Copy link
Contributor

rhgills commented Sep 1, 2013

Very interesting. I was messing with something similar - a replaceSelector:withDefinition: method that would do so in a much, much uglier fashion. For the same reasons: I've been writing some tests where I want to exercise a class along with its collaborators, but want to mock out the network interface to a third party API - and want to mock out some UI classes that are hard to test.

I like this idea.

Some things I learned from my attempt:

  • putting this method on the assembly was a bad, bad idea, with the class level state therein.

@jasperblues
Copy link
Member Author

I'm thinking something like PatchMutator

[mutator patch:[assembly authenticationManger] withDefinition:somethingElse]

[mutator patch:[assembly authenticationManger] withObject:someObject]

and maybe:

[mutator patch:[assembly authenticationManger] withMock:^{shortCut way to define mock. . although the above object way would already work}];

@rhgills
Copy link
Contributor

rhgills commented Sep 4, 2013

(Added in 3bc3330)

@rhgills
Copy link
Contributor

rhgills commented Sep 4, 2013

Looks good - I noticed we don't yet have tests for patchDefinition:withObject: added in c234306. I wonder, will it work correctly both before and after a TyphoonBlockComponentFactory is initialized using the assembly (and thus before and after the applying of beforeAdvice to method names via swizzling)?

I think adding a withObject: method is a good idea, too– to avoid having to create a block that just returns a simple object when that is all you need to do. I'm less convinced that we need withDefinition:. Can you think of any reason why you'd want to bother creating a definition? Maybe you want to use a different definition on the assembly in place of another one? Or maybe you have a TestAssembly, with prefab definitions you use for testing that is compiled into the test target? This seems useful if your fake objects would benefit from being constructed by DI, but if your fake objects have dependencies they might be a bit too complicated.

@jasperblues
Copy link
Member Author

  • There is one test in TyphoonPatcherTests.m . . we probably need more.

I can think of one or two plausible reasons why you might need to switch in a definition, but haven't actually needed it myself yet. . . if you're concerned, let's leave it out, until we get a specific argument in favor of its utility.

@jasperblues
Copy link
Member Author

  • (void)test_allows_patching_out_a_component_with_a_mock
    {
    MiddleAgesAssembly* assembly = [MiddleAgesAssembly assembly];
    TyphoonComponentFactory* factory = [TyphoonBlockComponentFactory factoryWithAssembly:assembly];

    TyphoonPatcher* patcher = [[TyphoonPatcher alloc] init];
    [patcher patchDefinition:[assembly knight] withObject:^id // <------- LOOKA HERE!
    {
    Knight* mockKnight = mock([Knight class]);
    [given([mockKnight favoriteDamsels]) willReturn:@[
    @"Mary",
    @"Janezzz"
    ]];

    return mockKnight;
    

    }];

    [factory attachMutator:patcher];

    Knight* knight = [factory componentForKey:@"knight"];
    LogDebug(@"Damsels: %@", [knight favoriteDamsels]);

}

See the above 'LOOKA HERE' comment. . . I added the ability to inject using the assembly interface? Does it make it confusing when the interface is acting as as component recipes and when it is acting as the interface to a built component factory. .. Is there a way we can make this obvious/self documenting/intuitive?

@rhgills
Copy link
Contributor

rhgills commented Sep 4, 2013

Oh, I completely missed that - just saw in the changelog that one was added for patchDefinitionForKey: in the previous commit, but didn't see that the test was modified in c234306. Looks like it works then :)

Not concerned at all re: definition switching, more just coming up empty thinking about it. What are the cases you have in mind?

@rhgills
Copy link
Contributor

rhgills commented Sep 4, 2013

It is a little confusing, but at least there are two well defined separate objects; the assembly holds the definitions and is otherwise (mostly) a 'data structure', while the factory creates the objects.

The confusion seems to be the cost of the ease of use we gain from allowing you to use method calls instead of string keys. Code generation to create an essentially duplicate interface for the factory, which returns different types, is the 'solution' that comes to mind.

So long as you're careful to keep your assembly and your factory instances clearly named and separate, it seems obvious to me. But then again I've had awhile to figure all this out.

@jasperblues
Copy link
Member Author

Perhaps if there was a good book on Typhoon, it would be easy enough ;) . . we could dedicate half a chapter to it.

@rhgills
Copy link
Contributor

rhgills commented Sep 4, 2013

Haha, great idea! ;)

@jasperblues
Copy link
Member Author

Closing this as good enough for now.

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

2 participants