Where should unit tests for deployers-spi go?

[jaikiran]

I am working on https://jira.jboss.org/jira/browse/JBDEPLOY-193 to provide a new deployer. I don't see any unit tests currently in jboss-deployers-spi project. Where should these tests go? And are the current deployers in this project being tested some place else?

[kabirkhan]

probably in deployers-impl?

[alesj]

"jaikiran" wrote:
I am working on https://jira.jboss.org/jira/browse/JBDEPLOY-193 to provide a new deployer.

Where was this discussed in the first place?
(I do vaguely recall some discussion)
Or when did I give a green light for this? ;-)

"jaikiran" wrote:

I don't see any unit tests currently in jboss-deployers-spi project. Where should these tests go? And are the current deployers in this project being tested some place else?

spi is tested in impl --> deployers-impl

[jaikiran]

"alesj" wrote:

Where was this discussed in the first place?
(I do vaguely recall some discussion)
Or when did I give a green light for this? ;-)

There was no green light :) But there was a discussion here http://www.jboss.org/index.html?module=bb&op=viewtopic&t=156862#4237186. You have some different idea for that?

[alesj]

"jaikiran" wrote:

There was no green light :)

It's all fine by me, if you make it back compatible and do proper testing. ;-)

[jaikiran]

"alesj" wrote:

It's all fine by me, if you make it back compatible and do proper testing. ;-)

Thanks.
Yep, it will be backward compatible and will have tests :) .

[alesj]

"jaikiran" wrote:

Yep, it will be backward compatible and will have tests :) .

This needs to be changed / fixed / in the worst case reverted.

(1) The names are bad
* RealDeployerWithInput - how should I know which one to take; vs. AbstractRealDeployerWithInput
* ExtendedDeploymentVisitor - extended? .. what does it extend ... Nameable perhaps?

(2) Looking at the impl, it's actually usesless to have this ExtendedDeploymentVisitor.
This could easily be a contract of the deployer, to have an attachment name + old DeploymentVisitor.
As all you do is lookup certain attachment and pass it to visitor.
Dunno why this should be part of the visitor, or why new interface is required.

[alesj]

"alesj" wrote:

This needs to be changed / fixed / in the worst case reverted.

I'll go ahead and fix this.
Check later if it still suites you.

[alesj]

(3) Keep self pity comments out of MC code ;-)

 // I seriously have no idea about how the setInput()/setInputs()/addInput() API expect/work
 // The more i look into it, the more confused i am.
 // For some weird reason, this is expected to be done.

[jaikiran]

"alesj" wrote:
(3) Keep self pity comments out of MC code ;-)

That's a TODO until i really understand the setInput()/setInputs()/addInput() API and their expected behaviour :)

[jaikiran]

"alesj" wrote:

I'll go ahead and fix this.
Check later if it still suites you.

Sure, will take a look tomorrow.

[alesj]

"jaikiran" wrote:

That's a TODO until i really understand the setInput()/setInputs()/addInput() API and their expected behaviour :)

You mean that it's currently like this?
- http://www.sciencecartoonsplus.com/images/miracle3.gif
:-)

[jaikiran]

"alesj" wrote:

You mean that it's currently like this?
- http://www.sciencecartoonsplus.com/images/miracle3.gif
:-)

LOL :)

"jaikiran" wrote:

That's a TODO until i really understand the setInput()/setInputs()/addInput() API and their expected behaviour :)

http://www.jboss.org/index.html?module=bb&op=viewtopic&p=4238485