Feature processor testing

[flurin]

Better way to do release/project testing by having ReleaseTestCase and ProjectTestCase.

This also includes test and fix for #25

TODO:

• [ ] Rewrite git_branch_finalizer test to use ReleaseTestCase as well

[edwinvdgraaf]

Haven't had more time to closely look into it, will do later. :bowtie:

But the test clearly looks cleaner. :+1: I do experience them hard to comprehend. Why have chosen to use inheritance?

[flurin]

As that makes calling 'super' in setup and teardown possible. This is much harder to do with modules.

On 19 jun. 2015, at 12:35, Edwin van der Graaf notifications@github.com wrote:

Haven't had more time to closely look into it, will do later.

But the test clearly look cleaner.
I do experience them hard to comprehend. Why have chosen to use inheritance?

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

[edwinvdgraaf]

Ok so the call to release and build_path, feels somewhat magical. E.g:

https://github.com/DigitPaint/roger/commit/65f26dcc2956e52f455af5a6830e11d570a30440#diff-70bf37612945691e9809d48dbe878317R12

But build_path is a part of release right? So release.build_path ?

Is it possible and perhaps worth considering to create a @release = Roger::ReleaseMock.new and teardown to do @release.teardown? While it makes accessing build_path a bit more of a drag, it is clearer, how the objects are created. :satisfied:

[flurin]

Nope build_path is self.build_path which is a method from the superclass (ReleaseTestCase). This is a classical class inheritance.

Using @release is bad practice as the superclass controls that variable. You should therefor only interact with the exposed interface (the accessor).

On 19 jun. 2015, at 14:53, Edwin van der Graaf notifications@github.com wrote:

Ok so the call to release and build_path, feels somewhat magical. E.g:

65f26dc#diff-70bf37612945691e9809d48dbe878317R12

But build_path is a part of release right? So release.build_path ?

Is it possible and perhaps worth considering to create a @release = Roger::ReleaseMock.new and teardown to do @release.teardown? While it makes accessing build_path a bit more of a drag, it is clearer, how the objects are created.

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

[flurin]

By the way: why the suggestion of a ReleaseMock?

[edwinvdgraaf]

Well, simply put I was trying to get rid of the inheritance, mainly to decrease complexity. I also see that I didn't mention that in my previous comment. Anyhow thats why.

I agree that @release would be undesirable when the superclass controls it, but once it is not a sublcass anymore?

[flurin]

But then you're just repeating code. Which is a pain to maintain.

Or maybe I do not understand what you mean?

On 19 jun. 2015, at 15:17, Edwin van der Graaf notifications@github.com wrote:

Well, simply put I was trying to get rid of the inheritance, mainly to decrease complexity. I also see that I didn't mention that in my previous comment. Anyhow thats why.

I agree that @release would be undesirable when the superclass controls it, but once it is not a sublcass anymore?

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

[flurin]

After discussion with @edwinvdgraaf I've come to the conclusion that the use of Mock objects that are subclasses of the actual objects have a lot of benefits. So I added an alternative to having subclasses of Test::Unit::TestCase. These can also be used outside of Roger in plugins for testing.

The module is called Testing which is a bit unintuitive but as Test is already taken by the Roger Test we have to make due.

Let me know what you think! If it's ok I'll remove the TestCase variant and squash those commits.

[flurin]

Also take a look at #31 which is a much better implementation (it includes only the Mock stuff). No need to review this mixed piece of garbage :)

[edwinvdgraaf]

Is everything in the #31 pull request or is there code that has to be extracted from this PR?

[flurin]

No, everything has been either cherry-picked to master, or put in #30, or otherwise put in #31

[edwinvdgraaf]

But that has been done? And this PR can be closed?

[flurin]

Yes