Buildr
  1. Buildr
  2. BUILDR-116

TestTask should include the main compile target in its dependencies, even when using non standard directories

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.3.3
    • Component/s: Test frameworks
    • Labels:
      None
    • Environment:
      Trunk revision 680924

      Description

      TestTask should include the main compile target in its dependencies, even when using non standard directories (e.g., 'src/java' instead of 'src/main/java').

      For instance, I have a project with java sources in 'src/java' and its junit tests in 'src/test'. The buildfile is roughly like this:
      <code>
      define 'my-project' do
      compile _('src/java')
      test.compile _('src/test')
      end
      </code>

      'buildr clean test' fails on test compilation because the main compile target (i.e., 'target/classes') is not in the TestTask's dependencies. The problem is that
      1. the main CompileTask tries to set its target directory during the 'before_define' phase, which is too early because it is before the "compile _('src/java')" line.
      2. the TestTask does not add the main compile target during the 'after_define' phase, because the main CompileTask still has no target at that time.

      1. BUILDR-116.lib.patch
        1 kB
        lacton
      2. BUILDR-116.spec.patch2
        0.7 kB
        lacton
      3. BUILDR-116.spec.patch
        0.8 kB
        lacton

        Activity

        Hide
        lacton added a comment -

        Sorry, my comments were confusing. I had not noticed that the comments I wrote while attaching a file were not shown as being related to the file.

        My comment "It supersedes the previous one" was about 'BUILDR-116.lib.patch'. I had written a previous patch I thought was quite low-quality. So I delete the first BUILDR-116.lib.patch and attached a new BUILDR-116.lib.patch that superseded it.

        On the other hand, the two spec patches were complementary. The first illustrates "TestTask should include the main compile target in its dependencies, even when using non standard directories". The second one illustrates the related but different issue "test compile target is not included in the test classpath when using a non standard directory".

        Next time, I will provide exactly one lib patch and one spec patch. It will be less confusing.

        Show
        lacton added a comment - Sorry, my comments were confusing. I had not noticed that the comments I wrote while attaching a file were not shown as being related to the file. My comment "It supersedes the previous one" was about ' BUILDR-116 .lib.patch'. I had written a previous patch I thought was quite low-quality. So I delete the first BUILDR-116 .lib.patch and attached a new BUILDR-116 .lib.patch that superseded it. On the other hand, the two spec patches were complementary. The first illustrates "TestTask should include the main compile target in its dependencies, even when using non standard directories". The second one illustrates the related but different issue "test compile target is not included in the test classpath when using a non standard directory". Next time, I will provide exactly one lib patch and one spec patch. It will be less confusing.
        Hide
        Assaf Arkin added a comment -

        You said the second one supersedes the first one, so I ignored the first one. Adding it now.

        Show
        Assaf Arkin added a comment - You said the second one supersedes the first one, so I ignored the first one. Adding it now.
        Hide
        lacton added a comment -

        Thank you for the commit, Assaf.

        I think you applied BUILDR-116.spec.patch2 but forgot BUILDR-116.spec.patch in the process.

        Show
        lacton added a comment - Thank you for the commit, Assaf. I think you applied BUILDR-116 .spec.patch2 but forgot BUILDR-116 .spec.patch in the process.
        Hide
        Assaf Arkin added a comment -

        Applied.

        Show
        Assaf Arkin added a comment - Applied.
        Hide
        lacton added a comment -

        Here is another patch. It supersedes the previous one.

        All existing specs and the two new ones are green with it.

        I like it much more, because it removes some duplication and shows the intent more clearly.

        Show
        lacton added a comment - Here is another patch. It supersedes the previous one. All existing specs and the two new ones are green with it. I like it much more, because it removes some duplication and shows the intent more clearly.
        Hide
        lacton added a comment -

        Another related issue is that the test compile target is not included in the test classpath when using a non standard directory. Refer to second spec patch.

        Show
        lacton added a comment - Another related issue is that the test compile target is not included in the test classpath when using a non standard directory. Refer to second spec patch.
        Hide
        lacton added a comment -

        This patch fixes the issue, but I do not like it for several reasons.

        1. it bypasses the 'protected' barrier by using the 'send' method

        2. it calls the 'associate_with' method, though the intention is not to associate the CompileTask, but just to force the auto-detect of the compiler and the target directory (i.e., only the last two lines of the associate_with method). Maybe CompileTask.compiler= could be called with a special value ':auto';

        3. I am not sure the TestTask's after_define is the best time to auto-detect the compiler one more time.

        4. I am not sure the CompileTask's before_define is the right time to auto-detect the compiler.

        Show
        lacton added a comment - This patch fixes the issue, but I do not like it for several reasons. 1. it bypasses the 'protected' barrier by using the 'send' method 2. it calls the 'associate_with' method, though the intention is not to associate the CompileTask, but just to force the auto-detect of the compiler and the target directory (i.e., only the last two lines of the associate_with method). Maybe CompileTask.compiler= could be called with a special value ':auto'; 3. I am not sure the TestTask's after_define is the best time to auto-detect the compiler one more time. 4. I am not sure the CompileTask's before_define is the right time to auto-detect the compiler.

          People

          • Assignee:
            Unassigned
            Reporter:
            lacton
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development