Inherited TestModules from parent classes

[Ambience]

I am trying to build a small integration test base that needs hooks
into @After, @Before, @BeforeClass, and @AfterClass. This prevents me
from wrapping it up in nice Module I can extend in each my tests. What
I need is for Jukito modules on parent tests to be installed as well.
For example:

public class MemPersistenceIntTest {
public static class Module extends TestModule {
protected void configureTest() {...}
}

@BeforeClass
public void initDb() {...}

@AfterClass
public void destroyDb() {...}
}

public class ListenerTest extends MemPersistenceIntTest {
public void testSometEmThings() {...}
}

Thoughts?

[Philippe Beaudoin]

I guess you could add a module to your subclass that extends the module in the parent... But please fill an issue for this, I believe it is a desirable feature.

Cheers,

   Philippe

[Tim Peierls]

Is it really desirable? I worry that the semantics of JukitoModule would start to get confusing. For example, how would one express the case where the parent module should not be installed? 

Philippe's suggestion is straightforward to write and (more importantly) to read.

--tim

[Philippe Beaudoin]

Yeah, maybe you're right. The simplest would probably be to finally support a way to define an external module and refer to it using an annotation...

[Tim Peierls]

You mean something like:

@RunWith(JukitoRunner.class)

@JukitoMainModule(MyJukitoModule.class)

public class MySomethingTest {

    // ...

}

and, for backward compatibility, treating the following as equivalent to the above (modulo the use of a nested class):

@RunWith(JukitoRunner.class)

public class MySomethingTest {

    public static class MyJukitoModule extends JukitoModule {

        // ...

    }

    // ...

}

?

That seems reasonable. You could make it more compact by having JukitoModule implement Runner (delegating Runner methods to an internal JukitoRunner):

@RunWith(MyJukitoModule.class)

public class MySomethingTest {

    // ...

}

--tim

[Philippe Beaudoin]

Yes, I definitely want to keep the embedded module class approach, which can sometimes be more compact. I like the syntax of the @RunWith. It's worth seeing how hard it would be to implement... The drawback I can see is that the @JukitoMainModule() approach could let you specify multiple modules in a list {...}.

   Philippe

[Tim Peierls]

On Thu, Sep 15, 2011 at 11:09 AM, Philippe Beaudoin <philippe...@gmail.com> wrote:

Yes, I definitely want to keep the embedded module class approach, which can sometimes be more compact. I like the syntax of the @RunWith. It's worth seeing how hard it would be to implement... The drawback I can see is that the @JukitoMainModule() approach could let you specify multiple modules in a list {...}.

You could provide both options. 

// For a single test module:

@RunWith(MyModule.class)

public class MyTest {

    ....

}

// For a small number of test modules

@RunWith(JukitoRunner.class)

@JukitoModules({MyModule1.class, MyModule2.class})

public class MyTest {

    ....

}

With really long lists of modules, it would probably be simpler and clearer to install them in an embedded module. 

@RunWith(JukitoRunner.class)

public class MyTest {

    public static class Module extends JukitoModule {

        protected void configureTest() {

            install(MyModule1.class);

            install(MyModule2.class);

            install(MyModule3.class);

            install(MyModule4.class);

            ...

        }

    }

    ....

}

The general rule would be to install any Jukito modules (or just TestModules?) from the @RunWith and @JukitoModules annotations, plus any nested JukitoModules.

--tim

[Philippe Beaudoin]

Yes, sounds good: compact, flexible...

   Philippe

[Ambience]

Sorry about the late response :D

While I think being able to annotate dependent modules is a fantastic
idea and a way to reduce boilerplate code when all you are doing is
installing other modules, I would push for inheritance from the parent
test. I would argue that it is semantically compatible with JUnit in
that if I register a @Before in my parent class, that gets called on
all child tests and there is no way for me to indicate that it
shouldn't be executed (to your point about excluding a module in some
subclasses). I would also argue that wanting to remove a module is a
"frenge" case and could be solved by simply excluding that module from
the parent or creating a deeper test hierarchy to facilitate the
permutations of modules.

In my specific case, the whole reason for the existence of the parent
test class is to establish a testing environment for tests that
inherit from it. To that point, it simply will not function with out
the installed module working in concert (providing dependencies to)
the test event listeners. Now, if some how I could annotate Jukito
modules with @Before @After and other test events, I wouldn't need a
parent test class at all and could just roll all of this into a single
module. But that seems like it would be an unclean mixing of concerns.

Maybe I have miss guessed the purpose of Jukito, but it seems like one
of the core tenets (and on of my chief reasons for using it) is
reduction of boilerplate. Requiring me to annotate every subclass with
the same annotations is boiler plate =(

Your initial suggestion is what I went with in the meantime but it
results in possibly confusing errors (by nature of being complex
dependency misses) if a programmer extends the test but forgets to
extend / install the module.

Great framework btw, I don't mean to just rag on the shortcomings :D

--Steve

On Sep 15, 9:32 am, Philippe Beaudoin <philippe.beaud...@gmail.com>
wrote:

[Philippe Beaudoin]

You make a strong case for it. My mind is not set... You may just have convinced me. A patch would most likely tip the scale in your favor. ;)

Cheers,

    Philippe

[Tim Peierls]

How about a spec before a patch? Create the injector from all TestModules defined as nested classes in the current test and all of its superclasses? All JukitoModules? All supertypes (including interfaces) or just superclasses? What about modules nested at depth two?

Forcing the modules to be listed explicitly in the test costs keystrokes, yes, but it prevents nasty surprises, too.

--tim

[Philippe Beaudoin]

I wonder if there could be a way of reporting, with the test, which modules have been used. This could be a nice and simple way to discover potential issues with modules defined in parent classes. Another approach could be to have an @InheritModule annotation, without parameters, documenting that you should inherit all the modules from your parent class. A few more keystrokes, but you don't have to look-up the module name in the parent...

I guess one goal should be to match Java's behavior regarding overriding/extension of static inner classes.

Thanks for this interesting discussion guys...

   Philippe

[Ambience]

Well, maybe I was too aggressive in saying this should be the the
default behavior. But I desperately want / need a way of defining a
test harness that requires zero configuration by implementers.
Basically I want to make a persistence integration test where coders
can depend on a memory database and a sensible hibernate configuration
to just work when they extend it.

I kind of feel like the whole point of abstraction / inheritance is
that you will be pulling in behaviors you may not fully understand, so
a little bit of ambiguity in which modules are being installed is
acceptable. Furthermore, simply by installing a module, or extending
from it, you take the "risk" that other modules you are not aware of
get installed.

However, if your concern is regression, where people are, for some
reason, stashing reusable modules in abstract parent tests that should
not / cannot be instantiated for all child classes, then ok :D I would
propose a similar solution but move the responsibly for defining
inheritability to the parent module, ala @InheritedTestModule or
@SuperModule (similar to JPAs @MappedSuperclass).

Or, you could make a test level annotation like
@RequiredModules(@RequireModule(value=MemoryPersistence,
inherited=true)). This could also be used as @RequireModule(...)
directly on the test if you only have one (there are plenty of APIs
that follow this format).

Or some other solution, but please, don't make me have to answer "why
am I getting null pointers in my persistence tests" questions from my
developers. Also, I can do this with AtUnit but I am trying to justify
switching to Jukito =\

On Sep 16, 7:07 am, Philippe Beaudoin <philippe.beaud...@gmail.com>
wrote:

> I wonder if there could be a way of reporting, with the test, which modules
> have been used. This could be a nice and simple way to discover potential
> issues with modules defined in parent classes. Another approach could be to
> have an @InheritModule annotation, without parameters, documenting that you
> should inherit all the modules from your parent class. A few more
> keystrokes, but you don't have to look-up the module name in the parent...
>
> I guess one goal should be to match Java's behavior regarding
> overriding/extension of static inner classes.
>
> Thanks for this interesting discussion guys...
>
>    Philippe
>
>
>
>
>
>
>
> On Fri, Sep 16, 2011 at 6:56 AM, Tim Peierls <t...@peierls.net> wrote:
> > How about a spec before a patch? Create the injector from all TestModules
> > defined as nested classes in the current test and all of its superclasses?
> > All JukitoModules? All supertypes (including interfaces) or just
> > superclasses? What about modules nested at depth two?
>
> > Forcing the modules to be listed explicitly in the test costs keystrokes,
> > yes, but it prevents nasty surprises, too.
>
> > --tim
>
> > On Thu, Sep 15, 2011 at 8:35 PM, Philippe Beaudoin <
> > philippe.beaud...@gmail.com> wrote:
>
> >> You make a strong case for it. My mind is not set... You may just have
> >> convinced me. A patch would most likely tip the scale in your favor. ;)
>
> >> Cheers,
>
> >>     Philippe
>

[Steve Skrla]

As for writing the patch, it will be a few weeks before I could
attempt one as we are in crunch mode, but I would love to contribute.

[Philippe Beaudoin]

You're more than welcome to! I don't have much time myself, but I think we should first go with a "inherit by default" and then we see if we should care about regression.

   Philippe

[Tim Peierls]

If someone does take this on, I'd like to plead for the default being not to inherit, because otherwise it could break existing test code.

At one point I had an abstract test superclass with a nested module M that installed some common things that some, not all, of the concrete test subclasses wanted installed. Then each subclass could install M or not with one line. 

Installing M by default would silently add M's bindings once in the subclasses that didn't want them, and twice in the ones that did, which might or might not cause a provision time error. 

I don't have that structure now, but it's not too hard to imagine that someone might.

--tim

[Philippe Beaudoin]

I agree with you Tim. Although I also appreciate the need for a single-point-of-control in the parent class. Things we could strive to support:

1) A module defined in the current way in the parent is NOT automatically inherited in the children

2) Children have ways to "install all parent modules"

3) Parents have a way to say "children inherit this module by default"

4) Children have a way to say "I do not want to inherit that module"

5) Children have a way to install specific modules

I'm not advocating in favor of ALL of these. Moreover, some are already doable today (ie, (5) with a call to install()). The question is really, which are useful, which document the behavior better, which requires too much boilerplate, which is best done in other ways (i.e. by rearchitecturing the inheritance graph)...

Anyway, food for thought.

   Philippe

[scl]

I started to use Jukito in my current project and I really like it.
As a developer grown up with Java 1.4 I might be a little old fashioned but I strongly like the possibility to pull up common behavior in a super class. Not in order to hide details but to prevent the so dared copy-paste code.

Therefore I d'like to vote in favor of a solution of which includes the following:

1) A module defined in the current way in the parent is NOT automatically inherited in the children

3) Parents have a way to say "children inherit this module by default"

5) Children have a way to install specific modules

I oppose the following:

2) Children have ways to "install all parent modules"

  - Inheritance is an automatism extending a class is all should do in order to be able to use the modules defined in the super class.

4) Children have a way to say "I do not want to inherit that module"

  - If you have the case that you want to inherit only a subset of the modules defined in a super class then the super class has too many responsibilities. Its time to either split the class in two separate classes or pull up the more general modules in an even higher super class and then extend from this new class.
  - There is also no possibility to skip an @Before, @BeforeClass, @After, or @AfterClass defined in a super class.


Therefore I d'like to propose the following approach:

Adding a new Annotation which can be used to annotate public static methods which return a Module.
All Modules returned by such annotated methods in the current test class or any of its super classes are automatically installed.
Mixing the current way with the inner static class and the new way with the annotated methods will cause a test failure for every test in the class.
Using the current way without mixing it with the new way will continue to work as it is right now.

Example:

public class TestBase {

@ModuleForTest
public static Module baseModule() {

// returns a module

}

}

public class Test1 extends BaseTest {

@ModuleForTest
public static Module testSpecificModule() {

// returns a module

}

@Test
public void someTest() {

// both modules from baseModule() and testSpecificModule() are installed.

}

}

public class Test2 extends BaseTest {

public static class Module extends JukitoModule {

// module definition

}

@Test
public void someTest() {

// fails because Test2 defines a static inner class extending JukitoModule and extends BaseTest which has an method annotated with @ModuleForTest

}

}


I would be willing to provide a patch which implements the proposed solution. I won't have time this week but could start next week.
I'm also open for other suggestions or ideas. Especially since I'm not an native English speaker I'm open to suggestions for the name of the proposed Annotation.

[Philippe Beaudoin]

This is my favorite solution to date. I believe the statu quo is just a bit too limiting. I'd welcome your patch.

Could we use simply @Module instead of @ModuleForTest?

Cheers,

   Philippe

[Tim Peierls]

I just posted a blog entry about this:

http://tembrel.blogspot.com/2011/12/overriding-common-bindings-with-test.html

--tim

[Philippe Beaudoin]

Interesting Tim. You're getting me wondering again... On one hand, the advantage is that the test documents its dependencies. On the other, the advantage is that you can entirely cut three-four lines in your subclasses. 

You say the second approach is "mysterious" but I'm not entirely sure I agree. The truth is that DI is "mysterious" by design. The idea of low coupling is to ensure nobody cares what they are connected to.

There is a drawback to this: when you have to figure out what you are actually connected to (usually for debugging) then you're a bit screwed. IMHO, though, documentation or extra boilerplate is not the solution. This problem should be solved with better tools. For example, in the case of Jukito, we should have a feature to trigger a complete report of the bindings for each test. In fact, I have a vague memory of building such a report tool at some point (is it in trunk?).

So, my opinion on this subject is: allow module inheritance. If someone wants to hide some bindings because the test shouldn't care, then it's a good way to do it while keeping the test super-short and readable.

Cheers,

   Philippe

[Tim Peierls]

On Tue, Dec 6, 2011 at 11:46 AM, Philippe Beaudoin <philippe...@gmail.com> wrote:

You say the second approach is "mysterious" but I'm not entirely sure I agree. The truth is that DI is "mysterious" by design. The idea of low coupling is to ensure nobody cares what they are connected to.

Sure, but at the point of injector creation you really want to have everything at your fingertips. A test class is the point of injector creation. There should always be some entry point or other analog of public static void main(). Right now a public static class extending JukitoModule is the entry point.

Thought experiment: If you have a regular Java class that doesn't have a public static void main, but that extends a class that does, do you expect the subclass to work as a main class?

 

There is a drawback to this: when you have to figure out what you are actually connected to (usually for debugging) then you're a bit screwed. IMHO, though, documentation or extra boilerplate is not the solution. This problem should be solved with better tools. For example, in the case of Jukito, we should have a feature to trigger a complete report of the bindings for each test. In fact, I have a vague memory of building such a report tool at some point (is it in trunk?).

(Aside: This is what Stage.TOOL is for, right?)

 

So, my opinion on this subject is: allow module inheritance. If someone wants to hide some bindings because the test shouldn't care, then it's a good way to do it while keeping the test super-short and readable.

Well, you're the boss, but I'm still queasy about auto-installing modules via slippery criteria like "is (annotated) nested static class of superclass of test".

What about having Jukito install any module returned by specially-annotated static method or field of the test class? Then you can get superclass bindings with:

class TestBase { /* defines static method commonBindings() */ }

public class MyConcreteTest extends TestBase {

    @Jukito static Module bindings() { return commonBindings(); }

    ...

}

or simply:

public class MyConcreteTest extends TestBase {

    @Jukito static final Module bindings = commonBindings();

    ...

}

That gives you the freedom to decouple common test bindings from common test class behavior:

class TestBase { /* defines common methods used in tests */ }

class TestBindings { /* defines common bindings for tests */ }

public class MyConcreteTest extends TestBase {

    @Jukito static Module main() { return TestBindings.commonBindings(); }

    ...

    // call TestBase methods in test methods

}

(Aside: At any rate, don't use @Module as an annotation; you don't want to risk ambiguity with com.google.inject.Module.)

For one shots:

public class MyConcreteTest {

    @Jukito static final Module MAIN = new TestModule() {

        protected configureTests() {

            ...

        }

    };

    ...

}

And you can still support the current style by treating:

public class MyConcreteTest {

    public static class Mod extends JukitoModule {

        protected void configureTests() { 

            ...

        }

    }

    ...

}

exactly as if it were:

public class MyConcreteTest {

    @Jukito public static final Module MOD = new JukitoModule() {

        protected void configureTests() { 

            ...

        }

    };

    ...

}

No looking in superclasses for the @Jukito annotation; if you want bindings from above, ask for them explicitly:

public class MyConcreteTest extends TestBase {

    @Jukito static final Module MAIN = Modules.override(commonBindings()).with(

        new AbstractModule() {

            protected void configure() {

                ...

            }

        }

    );

    ...

}

--tim

[Steve Skrla]

Maybe I am dense but this assumes you are frequently overwriting bindings. If that is the case then your method based solution is fine and more importantly compatible with inheritable modules so long as they are explicitly annotated to be inheritable. 

Which brings me to my question: in what way are inheritable annotated modules destructive to you or any one testing like you. More to the point, why shouldn't the rest of us be allowed to structure our tests the way we would like? It really seems like a question of personal style at this point as there are compelling arguments why you would / wouldn't use such a feature depending on your mental model.

There are plenty of features in hibernate, spring, your framework of choice that I would never use and may not even consider good ideas(tm) but I wouldn't lobby for preventing other developers from using them. I can understand trying to prevent feature bloat and striving for consistency, and I admit I am bias here, but I do not believe this request is bloat or inconsistant.

Also, the static method / field solution is just an annotated module with more boiler plate as Module.configure is basically the method you described.

--Steve

[Tim Peierls]

On Tue, Dec 6, 2011 at 2:25 PM, Steve Skrla <steve...@gmail.com> wrote:

Maybe I am dense but this assumes you are frequently overwriting bindings. If that is the case then your method based solution is fine and more importantly compatible with inheritable modules so long as they are explicitly annotated to be inheritable. 

Which brings me to my question: in what way are inheritable annotated modules destructive to you or any one testing like you.

I believe there is potential for confusion if Jukito automatically installs modules according to a criterion that involves the inheritance hierarchy of a test class. 

In standard Guice usage, the modules that go into an injector are explicitly handed over at injector creation time. This is a good thing: You can look at the injector creation point and see what's being used. 

In Jukito, each invocation of a method of test class is a little program; it gets its own injector, created from its own set of modules. Currently that set of modules is of cardinality zero or one, depending on whether you define a public static nested class extending JukitoModule. To use more modules, you can install them in the JukitoModule. Like "standard" Guice, you have one place to look.

If the annotated module feature is added, people will use it, and they will unwittingly give up the single-point listing and control of which modules are going into the test. A lot of the time that won't be a problem. My concern is that, having set up an inheritance structure that works well, some users will subsequently find that they need to override bindings in one place, and they'll be stuck. More likely it will be a different user, handed test code by the person who set up the structure, who comes across the need to override a binding. That new person can't go to a single place to do the overriding and can't use the class structure as is. It won't help to have others saying cheerfully, "Oh, if you needed to override things, you shouldn't have structured it that way in first place."

 

More to the point, why shouldn't the rest of us be allowed to structure our tests the way we would like?

You already are allowed to structure your tests the way you'd like. This is about the best way to reduce the amount of boilerplate needed to express your desired structure without making things more complicated for others.

 

It really seems like a question of personal style at this point as there are compelling arguments why you would / wouldn't use such a feature depending on your mental model.

I'm not worried about those who would decide not to use the feature. I'm worried about those who (understandably) haven't thought about the issue, who see the feature and, because it seems cool, decide to use it and then have problems.

 

There are plenty of features in hibernate, spring, your framework of choice that I would never use and may not even consider good ideas(tm) but I wouldn't lobby for preventing other developers from using them.

I'm not lobbying against the use of an existing feature. I'm trying to discuss the hidden potential drawbacks of a proposed feature.

 

I can understand trying to prevent feature bloat and striving for consistency, and I admit I am bias here, but I do not believe this request is bloat or inconsistant.

I'm not talking about bloat or consistency. It's possible that I'm overstating the risk of adding this feature, but I do think it's worth discussing.

 

Also, the static method / field solution is just an annotated module with more boiler plate as Module.configure is basically the method you described.

Not sure what you mean, but I'm not claiming to be original. My concern is finding a way to avoid semantics that involve looking in superclasses for test behavior.

--tim

[Steve Skrla]

In standard Guice usage, the modules that go into an injector are explicitly handed over at injector creation time. This is a good thing: You can look at the injector creation point and see what's being used. 

 

Single spring.xml files are also VERY explicit about what is being injected. Unless you are using a single module, some degree of code exploration is required to see what each sub module is providing. Most configurations are complex enough that there is even an API for generating graphs of your runtime injector configuration. This is another philosophical difference however and should probably be left for another discussion. My shop uses annotation based component scanning which would probably make you tear your hair out ;)

You already are allowed to structure your tests the way you'd like. This is about the best way to reduce the amount of boilerplate needed to express your desired structure without making things more complicated for others.

 

Requiring base class consumers to have configuration boilerplate both complicates the test class and increases said boilerplate. Maybe it's a question of semantics, and I don't have a strong case for my definition here, but I consider the necessary configuration of subclasses part of the 'structure'.

 

I'm not worried about those who would decide not to use the feature. I'm worried about those who (understandably) haven't thought about the issue, who see the feature and, because it seems cool, decide to use it and then have problems.

And those of us who have and decide the drawbacks are acceptable? A tool that does something well within a limited scope is still a good tool.

 

Not sure what you mean, but I'm not claiming to be original. My concern is finding a way to avoid semantics that involve looking in superclasses for test behavior.

I guess that's the crux of the argument, I (we?) find inherited behavior desirable. I agree that "restructure your classes" is not always an answer and that there is a possibility that some other developer get's backed into a corner. But this is a much longer and possibly unrelated discussion as there are ways to mitigate this and if all else fails the inheritance model could be augmented in a way that allows for overwriting. @TestModuleOverride(TargetedModuleToOverride.class) {function, field, or class def} for example.

I will take succinct readable code over configuration clutter for the sake of explicitness any day. This is, of corse, personal preference. 

I am more concerned with testing code function in a standard configuration then permutations of configurations 99% of the time.

[Tim Peierls]

On Tue, Dec 6, 2011 at 4:59 PM, Steve Skrla <steve...@gmail.com> wrote:

In standard Guice usage, the modules that go into an injector are explicitly handed over at injector creation time. This is a good thing: You can look at the injector creation point and see what's being used. 

 

Single spring.xml files are also VERY explicit about what is being injected. Unless you are using a single module, some degree of code exploration is required to see what each sub module is providing. Most configurations are complex enough that there is even an API for generating graphs of your runtime injector configuration. This is another philosophical difference however and should probably be left for another discussion. My shop uses annotation based component scanning which would probably make you tear your hair out ;)

Yup, and I know that (in spite of recommendations to the contrary from the Guice team) people routinely build deep module hierarchies (a installs b, c, d; b installs e, f, g; e installs h, i, j; etc.). And I've even flirted with scanning classpaths for annotated modules. These are often valid choices.

The key difference (w.r.t. our current discussion) is that it's a choice applied by the user of the DI framework, not by the framework itself. No one burned by an overly rigid/broken module-gathering facility can claim that the responsibility for fixing it lies with the Guice team. But adding the annotated module feature to Jukito, even if its use isn't required, puts some of the responsibility for any ensuing grief at the feet of those who added the feature.

The question is whether the amount of such grief is likely to be significant. I think so far people have been quick to dismiss the possibility (and you probably think I'm vastly overstating it). but it's worth exploring. The situation I'm worried about doesn't seem at all contrived or unlikely:

You have a TestBase class with methods commonly used by its concrete subclasses. You nest an annotated Module in TestBase that has all the bindings that the subclasses commonly need, and each subclass might or might not have it's own annotated nested Module for some extra stuff. 

Then you have a new test that, because it needs access to TestBase's methods, extends TestBase. When it turns out that you need to override one binding, you're in trouble. Maybe you could turn off all annotate-module finding and do it explicitly for that one subclass, but then you have to search the hierarchy manually to install the necessary modules -- and then you have maintain that list of "inherited" modules for that one test. Or you could take the original binding out of TestBase.Module, but now you have to put it in each of the other subclass-specific modules. Unhappiness! Anger! That darn Philippe! :-)

 

I'm not worried about those who would decide not to use the feature. I'm worried about those who (understandably) haven't thought about the issue, who see the feature and, because it seems cool, decide to use it and then have problems.

And those of us who have and decide the drawbacks are acceptable? A tool that does something well within a limited scope is still a good tool.

Sure, but when you add a feature you don't just consider the benefits to those who want it. You also have to consider the costs. If I've learned anything from my forays into language and library design, it's that people will tend to use new stuff just because it is there, without spending much time evaluating whether it makes sense for them. If you don't take these people into consideration, and they run into unanticipated problems later on, they will be unhappy and angry. You won't be able to say, "Well, you shouldn't have used that new feature." They'll run you out of town.

 

My concern is finding a way to avoid semantics that involve looking in superclasses for test behavior.

I guess that's the crux of the argument, I (we?) find inherited behavior desirable. I agree that "restructure your classes" is not always an answer and that there is a possibility that some other developer get's backed into a corner. But this is a much longer and possibly unrelated discussion as there are ways to mitigate this and if all else fails the inheritance model could be augmented in a way that allows for overwriting. @TestModuleOverride(TargetedModuleToOverride.class) {function, field, or class def} for example.

If the risk of introducing a feature is truly insignificant, then "we'll fix it in post" is a reasonable stance. It can't be the answer in general, because that would imply that any and every cool new feature should be added right away as long as there's someone to champion it.

 

I will take succinct readable code over configuration clutter for the sake of explicitness any day. This is, of course, personal preference. 

Hey, it's my preference, too! :-)

Seriously, I'm not bringing this up because of some deep-seated desire to write more boilerplate. I'm just saying that it's worth thinking a lot harder about the implications of adding something with such unusual and untried semantics.

What happened to putting an annotation with the class (or classes) to use at the top of the test class?

@RunWith(JukitoRunner.class)

@Jukito(TestBase.TestModule.class)

public class ConcreteTest extends TestBase {

    ...

}

That avoids inheritance issues and is pretty compact. I thought that's where we were headed until this most recent proposal.

 

I am more concerned with testing code function in a standard configuration then permutations of configurations 99% of the time.

I envy you. Lately a lot of my tests have been easiest to express as "standard configuration with one or two exceptions".

--tim

[Tim Peierls]

On Tue, Dec 6, 2011 at 4:59 PM, Steve Skrla <steve...@gmail.com> wrote:

I am more concerned with testing code function in a standard configuration then permutations of configurations 99% of the time.

 

On Tue, Dec 6, 2011 at 6:26 PM, Tim Peierls <t...@peierls.net> wrote:

What happened to putting an annotation with the class (or classes) to use at the top of the test class?

That avoids inheritance issues and is pretty compact. I thought that's where we were headed until this most recent proposal.

Another possibility came up in a private exchange with Steve: Make JukitoRunner subclassable with an extension point for providing the list of modules that will be handed to the injector (in addition to the "testModule" constructed through existing behavior).

To get the proposed behavior, you'd use this Runner instead of JukitoRunner:

public class InheritingJukitoRunner extends JukitoRunner {

    @Override protected Iterable<Module> provideExtraModules(Class<?> testClass) {

        // scan the inheritance hierarchy of testClass and add any nested (annotated?) static classes

        return ... results of scan ...

    }

}

Having a variant Runner in the Jukito toolkit feels like a safer way of providing new functionality than just adding the functionality directly to JukitoRunner. Those who don't want the extra behavior can stick with the plain JukitoRunner.

Would that make everyone happy? 

--tim

[Steve Skrla]

Thanks Tim, this would work perfectly for me.

[scl]

Sorry for starting a discussion and then not responding for so long.
December has been busier than expected.

Anyway I've been digging a little deeper into Guice and Jukito and also discussed my proposed changes and this discussion with a couple of coworkers.
I guess there is no perfect solution.

Extracting an Abstract runner class and then providing two concrete runners is simple. But it should be discouraged to write too many extension to the abstract runner.
I belive that the runner is a detail of the test which developer most of the time don't pay enough attention to. And the more runners there are the harder it will be to keep track of how each runner configures the injector which is used for running the test.

Therefore the design of the mechanism which allows to inherit an injector configuration and maybe modifying this configuration seems of much greater interest to me
In https://groups.google.com/d/topic/jukito/vDdcNKSsVCw/discussion Julian provided a patch to Jukito which allows to install Modules with an annotation. After writing several sample tests and some more discussions I came to the conclusion, that the Annotation approach is sufficient. Compared with my above proposal the amount of code which can be saved is neglectable. Therefore I will withdraw my proposed solution in favor of the solution by Julian.

In my current project I happen to stumble across the very rare case where I wanted to overwrite a binding from our common test module. Again I tried different approaches to solve this. One was refactoring the common module. It felt kind of funny because I was forced to ad a special sub classing for this single test. This didn't feel right. The class hierarchy suddenly had knowledge about the implementation of a specific sub class. In the current Jukito approach one can easily use Modules.overwrite() to replace one or more bindings for a specific test class. Another approach was to write a test specific module which does nothing more than to overwrite some bindings and then installing the resulting module. Which is a lot of code for not much action. The last approach was to introduce a second annotation which allows to overwrite inherited modules. This also didn't feel right. Adding a special annotation for every use case cannot be the solution. At the end one would just reinvent the API except of using classes with methods one would only use annotations.

A second shortcoming of the annotation based approach (and my proposed approach as well) is that it is not possible to decide which module should be use. Jukito provided two abstract modules which can be used to configure the injector for testing. TestModule and JukitoModule. The test will behave differently depending on which module is extended in the test class.

Finally I came up with a new approach:
Modules are basically a callback function. Guice will call configure(Binder) whenever it needs the informations stored in an Module.
One could write a Module which is configurable from the outside using the Guice Elements SPI. This would allow multiple actors to have an influence of the configuration which is stored in a module. In our scenario of test class hierarchies this would mean that a subclass could change the inherited configuration from a superclass. Such a configurable module would also be capable of behaving like a TestModule or like a JukitoModule based in its configuration.
I am well aware that writing such a module involves a lot more work than the annotation based approach. Still I think it is a very interesting approach and would be willing to invest some time into it.

I will add some example code to further outline what I like to propose:




public class BaseTestCase {

    @JukitoModuleProvider
    public static JukitoModule commonModules(JukitoModule module) {
        module.bind(InterfaceA.class).to(AImpl.clas);
        module.install(new DefaultModule());
       
        return module;
    }

    @Befor
    public void setUp(InterfaceA a) { ... }
   
    /* common test methods. */

}


// This class inherits the module unchanged from BaseTestCase
public class Test1 extends BaseTestCase {

    @Test
    public void fooTest(InterfaceA a) { ... }

}


// This class does the same as Test1. It is just more verbose
public class Test1A extends BaseTestCase {

    @JukitoModuleProvider
    public static JukitoModule commonModules(JukitoModule module) {
        return module;
    }

    @Test
    public void fooTest(InterfaceA a) { ... }

}


// This class extends the module from BaseTestCase
public class Test2 extends BaseTestCase {

    @JukitoModuleProvider
    public static JukitoModule specificModules(JukitoModule module) {
        module.bind(InterfaceB.class).toProvider(BProvider.class);
        module.overwrite(InterfaceA.class).with(AImplMock.class);
        return module;
    }

    @Test
    public void fooTest(InterfaceB b) { ... }

}


// This class ignores the module from BaseTestCase
public class Test2 extends BaseTestCase {

    @JukitoModuleProvider
    public static JukitoModule specificModules() {
        JukitoModule module = new JukitoModule();
        module.bind(InterfaceC.class).to(CImpl.class);
        return module;
    }

    @Test
    public void fooTest(InterfaceC c) { ... }

}


// This class does the same as Test2. But the runner can't tell by the
// method signature that the module provided by the super class is ignored.
public class Test2A extends BaseTestCase {

    @JukitoModuleProvider
    public static JukitoModule specificModules(JukitoModule module) {
        module = new JukitoModule();
        module.bind(InterfaceC.class).to(CImpl.class);
        return module;
    }

    @Test
    public void fooTest(InterfaceC c) { ... }

}

[scl]

any comment to my previous post? are you still interested in a new runner?

[scl]

I found some time to prepare the JukitoRunner to be expendable. A patch file with the changes is attached.

I also integrated the patch of Julian (see: https://groups.google.com/d/topic/jukito/vDdcNKSsVCw/discussion) into the patch (very last commit)