Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo

kemitix
Hi,

I'd like to contribute to karaf, initially by writing unit tests and then
refactoring the AssemblyMojo class in karaf-maven-plugin.

Just wanted to check that this wasn't something that anyone else was
working on before I started, or that there wasn't some other reason not to
do so.

Regards
Paul Campbell
http://github.com/kemitix
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo

jbonofre
Hi Paul,

Great ! Looking forward for a pull request about that.

I don't think anyone is working on this area right now.

Regards
JB

On 04/11/2017 08:06 AM, Paul Campbell wrote:

> Hi,
>
> I'd like to contribute to karaf, initially by writing unit tests and then
> refactoring the AssemblyMojo class in karaf-maven-plugin.
>
> Just wanted to check that this wasn't something that anyone else was
> working on before I started, or that there wasn't some other reason not to
> do so.
>
> Regards
> Paul Campbell
> http://github.com/kemitix
>

--
Jean-Baptiste Onofré
[hidden email]
http://blog.nanthrax.net
Talend - http://www.talend.com
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo

kemitix
Hi,

Trying to add the maven-plugin-test-harness requires Maven 3.2.4+, so I'm looking at upgrading Maven within the tooling module unless anyone has an objection.

Paul Campbell
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo

jbonofre
No problem for me. We just have to update the enforcer rule.

Regards
JB

On 04/12/2017 07:51 PM, kemitix wrote:

> Hi,
>
> Trying to add the maven-plugin-test-harness requires Maven 3.2.4+, so I'm
> looking at upgrading Maven within the tooling module unless anyone has an
> objection.
>
> Paul Campbell
>
>
>
> --
> View this message in context: http://karaf.922171.n3.nabble.com/Unit-tests-and-refactoring-karaf-maven-plugin-AssemblyMojo-tp4050098p4050109.html
> Sent from the Karaf - Dev mailing list archive at Nabble.com.
>

--
Jean-Baptiste Onofré
[hidden email]
http://blog.nanthrax.net
Talend - http://www.talend.com
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo

kemitix
Hi Jean-Baptiste,

Great. Enforcer has requireMavenVersion 3.3.9,4 already so I don't think it's caring about what dependencies are used within the module. Tooling is currently using Maven 3.0.3.

Is there a reason to keep the Dependency30Helper class in tooling.utils? Once the tooling module is upgraded to not use Maven 3.0 is there a reason why would this class needs to stay? Enforcer appears to already block use by 3.0 on the command line. I've not use Enforcer before, so I may be misunderstanding what it does.

Dependency31Helper can then be rewritten to use the Maven API directly, as noted in the javadoc.

If there's no reason to keep the 3.0 helper, I'll delete it rather than rewriting it to use reflection.

... then I can get back to writing tests for AssemblyMojo. :)

Regard
Paul Campbell
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo

jbonofre
Hi,

The DependencyHelper is able to deal with Maven/Aether dependency backend.
Upgrading will allow to use Maven dependency directly.

Regards
JB

On 04/12/2017 08:20 PM, kemitix wrote:

> Hi Jean-Baptiste,
>
> Great. Enforcer has requireMavenVersion 3.3.9,4 already so I don't think
> it's caring about what dependencies are used within the module. Tooling is
> currently using Maven 3.0.3.
>
> Is there a reason to keep the Dependency30Helper class in tooling.utils?
> Once the tooling module is upgraded to not use Maven 3.0 is there a reason
> why would this class needs to stay? Enforcer appears to already block use by
> 3.0 on the command line. I've not use Enforcer before, so I may be
> misunderstanding what it does.
>
> Dependency31Helper can then be rewritten to use the Maven API directly, as
> noted in the javadoc.
>
> If there's no reason to keep the 3.0 helper, I'll delete it rather than
> rewriting it to use reflection.
>
> ... then I can get back to writing tests for AssemblyMojo. :)
>
> Regard
> Paul Campbell
>
>
>
> --
> View this message in context: http://karaf.922171.n3.nabble.com/Unit-tests-and-refactoring-karaf-maven-plugin-AssemblyMojo-tp4050098p4050111.html
> Sent from the Karaf - Dev mailing list archive at Nabble.com.
>

--
Jean-Baptiste Onofré
[hidden email]
http://blog.nanthrax.net
Talend - http://www.talend.com
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RE: Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo

kemitix

Hi,

Here is a progress update on my work with AssemblyMojo.

I've been writing some exploratory tests to lock down the current behaviour of the AssemblyMojo.

1/ Split AssemblyMojo into a parameter/configuration class and a behaviour class

I have extracted the doExecute() method and supporting methods out of AssemblyMojo and into AssemblyMojoExec. (That probably needs a better name: AssemblyService?). This left all the @Parameter fields in AssemblyMojo, which is now passed as a single parameter object: AssemblyMojoExec.doExecute(AssemblyMojo mojo). This gives a clean separation between capturing 39 @Parameters and performing the actual assembly.

2/ Upgraded Maven in the tooling package to 3.5.0

This allows simplification of accessing the Maven API and avoids the need to use reflection. Upgrading the rest of karaf is outside the current scope.

3/ Added jacoco for code coverage, without any minimum coverage requirements

Jacoco is licensed under Eclipse Public License Version 1.0.

4/ Added mockito for test support

Mockito is licensed under M.I.T. License.

5/ Added hamcrest for test support

Hamcrest is licensed under BSD License.

6/ Write tests for AssemblyMojoExec covering 86% of branches.

The remaining 14% of branches are in unreachable File IO or Reflection exceptions and in switch statements.

7/ Identify unused @Parameters

The following @Parameter fields in AssemblyMojo are unused and could be removed without affecting the tests.
 
* featuresCfgFile
* startupPropertiesFile​​​​​​​
* systemDirectory
* defaultStartLevel
* blacklistedRepositories
* blacklistedFeatures
* startupBundles
* bootBundles
* installedBundles
* blacklistedBundles
* blacklistedProfiles

Is there any reason to keep any of these @Parameters? They are configuration items used by other Mojos. AssemblyMojo should really only ask for the @Parameters that it uses.

---

This current state is in https://github.com/kemitix/karaf/tree/assembly-mojo-test-coverage

Going on from here, I will be working in the assembly-mojo-refactoring branch to clean up and simplify the mojo, using the AssemblyMojoExecTest to ensure that behaviour remains consistent.
 
Feedback welcome.
 
--
Paul [W] Campbell
 

If you reply to this email, your message will be added to the discussion below:
http://karaf.922171.n3.nabble.com/Unit-tests-and-refactoring-karaf-maven-plugin-AssemblyMojo-tp4050098p4050113.html
To unsubscribe from Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo, click here.
NAML
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo

jbonofre
Hi Paul,

Can I see a branch on github ?

Can you use Easymock instead of Mockito (as we use Easymock in other part of
Karaf) ?

The @Parameters you mentioned were used in previous versions to generate the
features and cfg file.

Regards
JB

On 04/18/2017 11:44 PM, Paul Campbell wrote:

>
> Hi,
>
> Here is a progress update on my work with AssemblyMojo.
>
> I've been writing some exploratory tests to lock down the current behaviour of
> the AssemblyMojo.
>
> 1/ Split AssemblyMojo into a parameter/configuration class and a behaviour class
>
> I have extracted the doExecute() method and supporting methods out of
> AssemblyMojo and into AssemblyMojoExec. (That probably needs a better name:
> AssemblyService?). This left all the @Parameter fields in AssemblyMojo, which is
> now passed as a single parameter object: AssemblyMojoExec.doExecute(AssemblyMojo
> mojo). This gives a clean separation between capturing 39 @Parameters and
> performing the actual assembly.
>
> 2/ Upgraded Maven in the tooling package to 3.5.0
>
> This allows simplification of accessing the Maven API and avoids the need to use
> reflection. Upgrading the rest of karaf is outside the current scope.
>
> 3/ Added jacoco for code coverage, without any minimum coverage requirements
>
> Jacoco is licensed under Eclipse Public License Version 1.0.
>
> 4/ Added mockito for test support
>
> Mockito is licensed under M.I.T. License.
>
> 5/ Added hamcrest for test support
>
> Hamcrest is licensed under BSD License.
>
> 6/ Write tests for AssemblyMojoExec covering 86% of branches.
>
> The remaining 14% of branches are in unreachable File IO or Reflection
> exceptions and in switch statements.
>
> 7/ Identify unused @Parameters
>
> The following @Parameter fields in AssemblyMojo are unused and could be removed
> without affecting the tests.
>
> * featuresCfgFile
> * startupPropertiesFile​​​​​​​
> * systemDirectory
> * defaultStartLevel
> * blacklistedRepositories
> * blacklistedFeatures
> * startupBundles
> * bootBundles
> * installedBundles
> * blacklistedBundles
> * blacklistedProfiles
>
> Is there any reason to keep any of these @Parameters? They are configuration
> items used by other Mojos. AssemblyMojo should really only ask for the
> @Parameters that it uses.
>
> ---
>
> This current state is
> in https://github.com/kemitix/karaf/tree/assembly-mojo-test-coverage
>
> Going on from here, I will be working in the assembly-mojo-refactoring branch to
> clean up and simplify the mojo, using the AssemblyMojoExecTest to ensure that
> behaviour remains consistent.
>
> Feedback welcome.
>
> --
> Paul [W] Campbell
>
> --------------------------------------------------------------------------------
> If you reply to this email, your message will be added to the discussion below:
> http://karaf.922171.n3.nabble.com/Unit-tests-and-refactoring-karaf-maven-plugin-AssemblyMojo-tp4050098p4050113.html
>
> To unsubscribe from Unit tests and refactoring - karaf-maven-plugin -
> AssemblyMojo, click here
> <
>
> NAML
> <
http://karaf.922171.n3.nabble.com/template/NamlServlet.jtp?macro=macro_viewer&id=instant_html%21nabble%3Aemail.naml&base=nabble.naml.namespaces.BasicNamespace-nabble.view.web.template.NabbleNamespace-nabble.view.web.template.NodeNamespace&breadcrumbs=notify_subscribers%21nabble%3Aemail.naml-instant_emails%21nabble%3Aemail.naml-send_instant_email%21nabble%3Aemail.naml>
>

--
Jean-Baptiste Onofré
[hidden email]
http://blog.nanthrax.net
Talend - http://www.talend.com
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RE: Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo

kemitix
Hi JB,

The branch is at  https://github.com/kemitix/karaf/tree/assembly-mojo-test-coverage 

I'll need to do some reading on Easymock as I've not used it before.
 
--
Paul [W] Campbell
 
 

From: [hidden email]
Sent: Wed, 19 Apr 2017 6:42 AM
To: [hidden email]
Subject: Re: Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo
Hi Paul,

Can I see a branch on github ?

Can you use Easymock instead of Mockito (as we use Easymock in other part of
Karaf) ?

The @Parameters you mentioned were used in previous versions to generate the
features and cfg file.

Regards
JB

On 04/18/2017 11:44 PM, Paul Campbell wrote:

>
> Hi,
>
> Here is a progress update on my work with AssemblyMojo.
>
> I've been writing some exploratory tests to lock down the current behaviour of
> the AssemblyMojo.
>
> 1/ Split AssemblyMojo into a parameter/configuration class and a behaviour class
>
> I have extracted the doExecute() method and supporting methods out of
> AssemblyMojo and into AssemblyMojoExec. (That probably needs a better name:
> AssemblyService?). This left all the @Parameter fields in AssemblyMojo, which is
> now passed as a single parameter object: AssemblyMojoExec.doExecute(AssemblyMojo
> mojo). This gives a clean separation between capturing 39 @Parameters and
> performing the actual assembly.
>
> 2/ Upgraded Maven in the tooling package to 3.5.0
>
> This allows simplification of accessing the Maven API and avoids the need to use
> reflection. Upgrading the rest of karaf is outside the current scope.
>
> 3/ Added jacoco for code coverage, without any minimum coverage requirements
>
> Jacoco is licensed under Eclipse Public License Version 1.0.
>
> 4/ Added mockito for test support
>
> Mockito is licensed under M.I.T. License.
>
> 5/ Added hamcrest for test support
>
> Hamcrest is licensed under BSD License.
>
> 6/ Write tests for AssemblyMojoExec covering 86% of branches.
>
> The remaining 14% of branches are in unreachable File IO or Reflection
> exceptions and in switch statements.
>
> 7/ Identify unused @Parameters
>
> The following @Parameter fields in AssemblyMojo are unused and could be removed
> without affecting the tests.
>
> * featuresCfgFile
> * startupPropertiesFile
> * systemDirectory
> * defaultStartLevel
> * blacklistedRepositories
> * blacklistedFeatures
> * startupBundles
> * bootBundles
> * installedBundles
> * blacklistedBundles
> * blacklistedProfiles
>
> Is there any reason to keep any of these @Parameters? They are configuration
> items used by other Mojos. AssemblyMojo should really only ask for the
> @Parameters that it uses.
>
> ---
>
> This current state is
> in https://github.com/kemitix/karaf/tree/assembly-mojo-test-coverage
>
> Going on from here, I will be working in the assembly-mojo-refactoring branch to
> clean up and simplify the mojo, using the AssemblyMojoExecTest to ensure that
> behaviour remains consistent.
>
> Feedback welcome.
>
> --
> Paul [W] Campbell
>
> --------------------------------------------------------------------------------
> If you reply to this email, your message will be added to the discussion below:
> http://karaf.922171.n3.nabble.com/Unit-tests-and-refactoring-karaf-maven-plugin-AssemblyMojo-tp4050098p4050113.html
>
> To unsubscribe from Unit tests and refactoring - karaf-maven-plugin -
> AssemblyMojo, click here
> <
>
> NAML
> <
http://karaf.922171.n3.nabble.com/template/NamlServlet.jtp?macro=macro_viewer&id=instant_html%21nabble%3Aemail.naml&base=nabble.naml.namespaces.BasicNamespace-nabble.view.web.template.NabbleNamespace-nabble.view.web.template.NodeNamespace&breadcrumbs=notify_subscribers%21nabble%3Aemail.naml-instant_emails%21nabble%3Aemail.naml-send_instant_email%21nabble%3Aemail.naml>
>
--
Jean-Baptiste Onofré
[hidden email]
http://blog.nanthrax.net
Talend - http://www.talend.com

 
If you reply to this email, your message will be added to the discussion below:
http://karaf.922171.n3.nabble.com/Unit-tests-and-refactoring-karaf-maven-plugin-AssemblyMojo-tp4050098p4050177.html
To unsubscribe from Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo, click here.
NAML
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo

jbonofre
Thanks, I gonna take a look.

Easymock is pretty similar to Mockito (Easymock.createMock() instead of
Mockito.mock()), etc.

Regards
JB

On 04/19/2017 07:55 AM, kemitix wrote:

> Hi JB,
>
> The branch is at  https://github.com/kemitix/karaf/tree/assembly-mojo-test-coverage
>
> I'll need to do some reading on Easymock as I've not used it before.
>
> --
> Paul [W] Campbell     From: jbonofre [via Karaf] <[hidden email]>
> Sent: Wed, 19 Apr 2017 6:42 AM
> To: kemitix <[hidden email]>
> Subject: Re: Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo
>
> Hi Paul,
>
> Can I see a branch on github ?
>
> Can you use Easymock instead of Mockito (as we use Easymock in other part of
> Karaf) ?
>
> The @Parameters you mentioned were used in previous versions to generate the
> features and cfg file.
>
> Regards
> JB
>
> On 04/18/2017 11:44 PM, Paul Campbell wrote:
>
>>
>> Hi,
>>
>> Here is a progress update on my work with AssemblyMojo.
>>
>> I've been writing some exploratory tests to lock down the current behaviour of
>> the AssemblyMojo.
>>
>> 1/ Split AssemblyMojo into a parameter/configuration class and a behaviour class
>>
>> I have extracted the doExecute() method and supporting methods out of
>> AssemblyMojo and into AssemblyMojoExec. (That probably needs a better name:
>> AssemblyService?). This left all the @Parameter fields in AssemblyMojo, which is
>> now passed as a single parameter object: AssemblyMojoExec.doExecute(AssemblyMojo
>> mojo). This gives a clean separation between capturing 39 @Parameters and
>> performing the actual assembly.
>>
>> 2/ Upgraded Maven in the tooling package to 3.5.0
>>
>> This allows simplification of accessing the Maven API and avoids the need to use
>> reflection. Upgrading the rest of karaf is outside the current scope.
>>
>> 3/ Added jacoco for code coverage, without any minimum coverage requirements
>>
>> Jacoco is licensed under Eclipse Public License Version 1.0.
>>
>> 4/ Added mockito for test support
>>
>> Mockito is licensed under M.I.T. License.
>>
>> 5/ Added hamcrest for test support
>>
>> Hamcrest is licensed under BSD License.
>>
>> 6/ Write tests for AssemblyMojoExec covering 86% of branches.
>>
>> The remaining 14% of branches are in unreachable File IO or Reflection
>> exceptions and in switch statements.
>>
>> 7/ Identify unused @Parameters
>>
>> The following @Parameter fields in AssemblyMojo are unused and could be removed
>> without affecting the tests.
>>
>> * featuresCfgFile
>> * startupPropertiesFile
>> * systemDirectory
>> * defaultStartLevel
>> * blacklistedRepositories
>> * blacklistedFeatures
>> * startupBundles
>> * bootBundles
>> * installedBundles
>> * blacklistedBundles
>> * blacklistedProfiles
>>
>> Is there any reason to keep any of these @Parameters? They are configuration
>> items used by other Mojos. AssemblyMojo should really only ask for the
>> @Parameters that it uses.
>>
>> ---
>>
>> This current state is
>> in https://github.com/kemitix/karaf/tree/assembly-mojo-test-coverage
>>
>> Going on from here, I will be working in the assembly-mojo-refactoring branch to
>> clean up and simplify the mojo, using the AssemblyMojoExecTest to ensure that
>> behaviour remains consistent.
>>
>> Feedback welcome.
>>
>> --
>> Paul [W] Campbell
>>
>> --------------------------------------------------------------------------------
>> If you reply to this email, your message will be added to the discussion below:
>> http://karaf.922171.n3.nabble.com/Unit-tests-and-refactoring-karaf-maven-plugin-AssemblyMojo-tp4050098p4050113.html
>>
>> To unsubscribe from Unit tests and refactoring - karaf-maven-plugin -
>> AssemblyMojo, click here
>> <
>>
>> NAML
>> < http://karaf.922171.n3.nabble.com/template/NamlServlet.jtp?macro=macro_viewer&id=instant_html%21nabble%3Aemail.naml&base=nabble.naml.namespaces.BasicNamespace-nabble.view.web.template.NabbleNamespace-nabble.view.web.template.NodeNamespace&breadcrumbs=notify_subscribers%21nabble%3Aemail.naml-instant_emails%21nabble%3Aemail.naml-send_instant_email%21nabble%3Aemail.naml>
>>
> --
> Jean-Baptiste Onofré
> [hidden email]
> http://blog.nanthrax.net
> Talend - http://www.talend.com
>
>   If you reply to this email, your message will be added to the discussion below: http://karaf.922171.n3.nabble.com/Unit-tests-and-refactoring-karaf-maven-plugin-AssemblyMojo-tp4050098p4050177.html To unsubscribe from Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo, click here.
> NAML
>
>
>
> --
> View this message in context: http://karaf.922171.n3.nabble.com/Unit-tests-and-refactoring-karaf-maven-plugin-AssemblyMojo-tp4050098p4050178.html
> Sent from the Karaf - Dev mailing list archive at Nabble.com.
>

--
Jean-Baptiste Onofré
[hidden email]
http://blog.nanthrax.net
Talend - http://www.talend.com
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo

kemitix
Hi,

Refactoring is pretty much done. https://github.com/kemitix/karaf/tree/assembly-mojo-refactoring

There are 8 classes that have been extracted from AssemblyMojo and the whole thing moved into an 'assembly' package.

Test coverage, using mockito, for all 9 classes is now 100%.

Now I just need to rewrite all the tests in easymock. I should hopefully find this to be doable now as I, hopefully, shouldn't need to use a Spy object or any argument captors.

--
Paul Campbell
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo

kemitix
Hi,

Branch: https://github.com/kemitix/karaf/tree/assembly-mojo-refactoring

I've finished refactoring into the assembly package the AssemblyMojo in the karaf-maven-plugin module.

There are unit tests providing 100% code coverage of the assembly package.

The original mockito-based test I had written has been adapted to use Easymock and a 'BuilderSpy' class. It has been renamed as AssemblyMojoSystemTest, and it too also provides 100% code coverage for the assembly package.

Currently, this work is based on the karaf-4.1.x branch. There have been no modifications to AssemblyMojo between karaf-4.1.x and master, so rebasing it onto master should not cause any problems.

What should I do next in order to progress this towards being merged with the main line?

Regards
Paul Campbell
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo

Guillaume Nodet-2
Thx for this work, better code coverage is really nice !
I have one question though, what's the benefit of splitting the
AssemblyMojo / AssemblyMojoExec ?
It leads to duplication of all the fields for no real gain to me ...
Most of the actual execution is already extracted in the Builder class, so
I'm not sure we'll benefit from another split.

2017-05-09 16:37 GMT+02:00 kemitix <[hidden email]>:

> Hi,
>
> Branch: https://github.com/kemitix/karaf/tree/assembly-mojo-refactoring
>
> I've finished refactoring into the assembly package the AssemblyMojo in the
> karaf-maven-plugin module.
>
> There are unit tests providing 100% code coverage of the assembly package.
>
> The original mockito-based test I had written has been adapted to use
> Easymock and a 'BuilderSpy' class. It has been renamed as
> AssemblyMojoSystemTest, and it too also provides 100% code coverage for the
> assembly package.
>
> Currently, this work is based on the karaf-4.1.x branch. There have been no
> modifications to AssemblyMojo between karaf-4.1.x and master, so rebasing
> it
> onto master should not cause any problems.
>
> What should I do next in order to progress this towards being merged with
> the main line?
>
> Regards
> Paul Campbell
>
>
>
> --
> View this message in context: http://karaf.922171.n3.nabble.
> com/Unit-tests-and-refactoring-karaf-maven-plugin-AssemblyMojo-
> tp4050098p4050322.html
> Sent from the Karaf - Dev mailing list archive at Nabble.com.
>



--
------------------------
Guillaume Nodet
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RE: Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo

kemitix
Hi,

If I recall, when I was starting this exercise, I needed the separation to conceptualise all the parameters apart from the behaviour to allow me to write the initial exploratory tests. It could probably be folded back in with only minor changes.

Regards
Paul Campbell
 
 

From: [hidden email]
Sent: Tue, 9 May 2017 3:48 PM
To: [hidden email]
Subject: Re: Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo
Thx for this work, better code coverage is really nice !
I have one question though, what's the benefit of splitting the
AssemblyMojo / AssemblyMojoExec ?
It leads to duplication of all the fields for no real gain to me ...
Most of the actual execution is already extracted in the Builder class, so
I'm not sure we'll benefit from another split.

2017-05-09 16:37 GMT+02:00 kemitix < [hidden email]>:

> Hi,
>
> Branch: https://github.com/kemitix/karaf/tree/assembly-mojo-refactoring
>
> I've finished refactoring into the assembly package the AssemblyMojo in the
> karaf-maven-plugin module.
>
> There are unit tests providing 100% code coverage of the assembly package.
>
> The original mockito-based test I had written has been adapted to use
> Easymock and a 'BuilderSpy' class. It has been renamed as
> AssemblyMojoSystemTest, and it too also provides 100% code coverage for the
> assembly package.
>
> Currently, this work is based on the karaf-4.1.x branch. There have been no
> modifications to AssemblyMojo between karaf-4.1.x and master, so rebasing
> it
> onto master should not cause any problems.
>
> What should I do next in order to progress this towards being merged with
> the main line?
>
> Regards
> Paul Campbell
>
>
>
> --
> View this message in context: http://karaf.922171.n3.nabble.
> com/Unit-tests-and-refactoring-karaf-maven-plugin-AssemblyMojo-
> tp4050098p4050322.html
> Sent from the Karaf - Dev mailing list archive at Nabble.com.
>


--
------------------------
Guillaume Nodet

 
If you reply to this email, your message will be added to the discussion below:
http://karaf.922171.n3.nabble.com/Unit-tests-and-refactoring-karaf-maven-plugin-AssemblyMojo-tp4050098p4050323.html
To unsubscribe from Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo, click here.
NAML
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Unit tests and refactoring - karaf-maven-plugin - AssemblyMojo

kemitix
In reply to this post by Guillaume Nodet-2
Hi,

I've folded AssemblyMojoExec back into AssemblyMojo and pushed the changes to the branch.

Regards
Paul Campbell
Loading...