Maven 1.x AspectJ Plugin

aspectj:test-compile goal should check for pom.build.unitTestSourceDirectory

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 3.1
  • Fix Version/s: 3.1.1
  • Labels:
    None
  • Environment:
    maven-1.0-rc4
  • Number of attachments :
    1

Description

The aspectj plugin should check for the presence of the unit test
source directory before calling aspectj:compile. This is useful in cases where you want to define your preGoal in your subproject's maven.xml and subprojects don't have any tests defined. Otherwise you'll get a error telling you the 'maven.test.compile.src.set' property doesn't exist. This is easy to add, just add the following check to the aspectj:test-compile goal:

<j:set var="testSourceDir" value="$

{pom.build.unitTestSourceDirectory}

"/>
<ant:available
property="hasTestDir"
file="$

{testSourceDir}

"/>

<j:if test="$

{shouldWeave == 'true' and hasTestDir != null and testSourceDir != ''}

">

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Chad Brandon added a comment -

"This is useful in cases where you want to define your preGoal in your subproject's maven.xml and subprojects don't have any tests defined."

Sorry I meant to say: This is useful in cases where you want to define your preGoal in your SUPER project's maven.xml and subprojects don't have any tests defined

Show
Chad Brandon added a comment - "This is useful in cases where you want to define your preGoal in your subproject's maven.xml and subprojects don't have any tests defined." Sorry I meant to say: This is useful in cases where you want to define your preGoal in your SUPER project's maven.xml and subprojects don't have any tests defined
Hide
Permalink
Chad Brandon added a comment -

Along the same lines, it should also add checks to make sure the
$

{pom.build.aspectSourceDirectory}

is not empty whenever it makes the check "aspectSourcesPresent == 'true'". If
the aspectSourceDirectory has NOT been defined in the POM, the property will be empty
(instead of null) therefore the ant:available task will falsely say the directory does exist and set the aspectSourcesPresent property to "true".

Show
Chad Brandon added a comment - Along the same lines, it should also add checks to make sure the $ {pom.build.aspectSourceDirectory} is not empty whenever it makes the check "aspectSourcesPresent == 'true'". If the aspectSourceDirectory has NOT been defined in the POM, the property will be empty (instead of null) therefore the ant:available task will falsely say the directory does exist and set the aspectSourcesPresent property to "true".
Hide
Permalink
Chad Brandon added a comment -

I'm attaching the updated plugin.jelly that has the fixes I mention
above.

Show
Chad Brandon added a comment - I'm attaching the updated plugin.jelly that has the fixes I mention above.
Hide
Permalink
Chad Brandon added a comment -

One more check needs to be added to the file I attached in my last comment, in this section below, you'll see I've added a check for "or aspectSourceDir == ''" to the <j:if> tag (where aspectSourceDir is pom.build.aspectSourceDirectory).

This needs to be here, or else the aspectLibraryPresent property will never be set to true (when there is no pom.build.aspectSourceDirectory defined in the POM), since as I've said in my previous comments aspectSourcesPresent will be true in this case.

<!-- If there aren't aspect sources check if there are aspect libraries -->
<j:if test="$

{aspectSourcesPresent == null or aspectSourceDir == ''}

">
<j:forEach var="artifact" items="$

{pom.artifacts}

">
<j:set var="dep" value="$

{artifact.dependency}

"/>
<j:if test="$

{dep.getProperty('aspectj.weaveWith')=='true' and dep.type=='jar'}

">
<j:set var="aspectLibrariesPresent" value="true"/>
<j:break/>
</j:if>
<j:if test="$

{dep.getProperty('aspectj.weaveInto')=='true' and dep.type=='jar'}

">
<j:set var="aspectLibrariesPresent" value="true"/>
<j:break/>
</j:if>
</j:forEach>
</j:if>

Show
Chad Brandon added a comment - One more check needs to be added to the file I attached in my last comment, in this section below, you'll see I've added a check for "or aspectSourceDir == ''" to the <j:if> tag (where aspectSourceDir is pom.build.aspectSourceDirectory). This needs to be here, or else the aspectLibraryPresent property will never be set to true (when there is no pom.build.aspectSourceDirectory defined in the POM), since as I've said in my previous comments aspectSourcesPresent will be true in this case. <!-- If there aren't aspect sources check if there are aspect libraries --> <j:if test="$ {aspectSourcesPresent == null or aspectSourceDir == ''} "> <j:forEach var="artifact" items="$ {pom.artifacts} "> <j:set var="dep" value="$ {artifact.dependency} "/> <j:if test="$ {dep.getProperty('aspectj.weaveWith')=='true' and dep.type=='jar'} "> <j:set var="aspectLibrariesPresent" value="true"/> <j:break/> </j:if> <j:if test="$ {dep.getProperty('aspectj.weaveInto')=='true' and dep.type=='jar'} "> <j:set var="aspectLibrariesPresent" value="true"/> <j:break/> </j:if> </j:forEach> </j:if>
Hide
Permalink
Carlos Sanchez added a comment -

Next time please submit patches in unified diff format.
It will be fixed faster.

Show
Carlos Sanchez added a comment - Next time please submit patches in unified diff format. It will be fixed faster.
Hide
Permalink
Chad Brandon added a comment -

Thanks for fixing it. Sure...I'll do that next time.

Show
Chad Brandon added a comment - Thanks for fixing it. Sure...I'll do that next time.

People

  • Assignee:
    Carlos Sanchez
    Reporter:
    Chad Brandon
Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved:

Time Tracking

Estimated:
10m
Original Estimate - 10 minutes
Remaining:
10m
Remaining Estimate - 10 minutes
Logged:
Not Specified
Time Spent - Not Specified