Maven 1.x AspectJ Plugin
  1. Maven 1.x AspectJ Plugin
  2. MPASPECTJ-11

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

        Hide
        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
        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
        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
        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
        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
        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
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

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