Details

    • Type: Sub-task Sub-task
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.5.0
    • Fix Version/s: 0.5.0
    • Labels:
      None

      Description

      [From Skype call on 12/20] s4-comm tests currently use file-based configurations - AssignmentFromFile, TopologyFromFile. Instead, they should change to using zookeeper-based configurations.

      Steps involved –
      1. Move org.apache.s4.fixtures from s4-core/test to s4-comm/test to avoid duplication of code
      2. Rewrite s4-comm/tests to use these fixtures.

      Issues –
      1. s4.fixtures uses some App-related code

      EDIT: This JIRA addresses only the Step 1 above. Step 2 should be a different JIRA

      1. S4-33.patch
        68 kB
        Karthik Kambatla

        Issue Links

          Activity

          Hide
          Karthik Kambatla added a comment -

          To make s4.fixtures accessible to s4-comm as well, I split s4.fixtures across s4-comm an s4-core. s4-core has the SocketAdapter and S4TestUtils. S4TestUtils extends TestUtils to implement forkS4App() and forkS4Node().

          When running tests from within Eclipse, this works absolutely fine. However, gradle test fails because it doesn't understand symbols.

          So, either s4-core should depend on s4-comm or the fixtures should move all the way to s4-base. Which one do you think is better?

          Show
          Karthik Kambatla added a comment - To make s4.fixtures accessible to s4-comm as well, I split s4.fixtures across s4-comm an s4-core. s4-core has the SocketAdapter and S4TestUtils. S4TestUtils extends TestUtils to implement forkS4App() and forkS4Node(). When running tests from within Eclipse, this works absolutely fine. However, gradle test fails because it doesn't understand symbols. So, either s4-core should depend on s4-comm or the fixtures should move all the way to s4-base. Which one do you think is better?
          Hide
          Matthieu Morel added a comment -

          You're right, only some of the fixtures need to be moved to s4-comm. Maybe you could rename util classes to CoreTestUtils and CommTestUtils.

          What do you mean by "doesn't understand symbols"?

          Doesn't s4-core already depends on s4-comm ? Anyway, it would be nicer to have zk dependencies only from s4-comm but really, it isn't a big deal if you introduce those dependencies for fixtures in s4-base, since it's only for tests.

          Show
          Matthieu Morel added a comment - You're right, only some of the fixtures need to be moved to s4-comm. Maybe you could rename util classes to CoreTestUtils and CommTestUtils. What do you mean by "doesn't understand symbols"? Doesn't s4-core already depends on s4-comm ? Anyway, it would be nicer to have zk dependencies only from s4-comm but really, it isn't a big deal if you introduce those dependencies for fixtures in s4-base, since it's only for tests.
          Hide
          Karthik Kambatla added a comment -

          To start with, I have refactored the TestUtils into CommTestUtils and CoreTestUtils.

          Now, when I run gradle :s4-core:test, I get the following output. It compiles fine in Eclipse. s4-core.gradle script does have s4-comm under its dependencies. I am kind of stuck and would really appreciate any help.

          The 'urls' property of the RepositoryHandler.mavenRepo() method is deprecated and will be removed in a future version of Gradle. You should use the 'url' property to define the core maven repository & the 'artifactUrls' property to define any additional artifact locations.
          The Configuration.getAllArtifactFiles() method is deprecated and will be removed in the next version of Gradle. You should use the getAllArtifacts().getFiles() method instead.
          :s4-base:compileJava UP-TO-DATE
          :s4-base:processResources UP-TO-DATE
          :s4-base:classes UP-TO-DATE
          :s4-base:jar UP-TO-DATE
          :s4-comm:compileJava UP-TO-DATE
          :s4-comm:processResources UP-TO-DATE
          :s4-comm:classes UP-TO-DATE
          :s4-comm:jar UP-TO-DATE
          :s4-core:compileJava UP-TO-DATE
          :s4-core:processResources UP-TO-DATE
          :s4-core:classes UP-TO-DATE
          :s4-core:compileTestJavaCoreTestUtils.java:5: package org.apache.s4.comm.fixtures does not exist
          import org.apache.s4.comm.fixtures.CommTestUtils;
          ^
          CoreTestUtils.java:15: cannot find symbol
          symbol: class CommTestUtils
          public class CoreTestUtils extends CommTestUtils {
          ^
          CoreTestUtils.java:18: cannot find symbol
          symbol : method forkProcess(java.lang.String,java.lang.String,java.lang.String)
          location: class org.apache.s4.fixtures.CoreTestUtils
          return forkProcess(App.class.getName(), moduleClass.getName(), appClass.getName());
          ^
          CoreTestUtils.java:22: cannot find symbol
          symbol : method forkProcess(java.lang.String,java.lang.String[])
          location: class org.apache.s4.fixtures.CoreTestUtils
          return forkProcess(Main.class.getName(), new String[] {});
          ^
          4 errors

          FAILURE: Build failed with an exception.

          • What went wrong:
            Execution failed for task ':s4-core:compileTestJava'.
            Cause: Compile failed; see the compiler error output for details.
          • Try:
            Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output.

          BUILD FAILED

          Total time: 19.816 secs

          Show
          Karthik Kambatla added a comment - To start with, I have refactored the TestUtils into CommTestUtils and CoreTestUtils. Now, when I run gradle :s4-core:test, I get the following output. It compiles fine in Eclipse. s4-core.gradle script does have s4-comm under its dependencies. I am kind of stuck and would really appreciate any help. The 'urls' property of the RepositoryHandler.mavenRepo() method is deprecated and will be removed in a future version of Gradle. You should use the 'url' property to define the core maven repository & the 'artifactUrls' property to define any additional artifact locations. The Configuration.getAllArtifactFiles() method is deprecated and will be removed in the next version of Gradle. You should use the getAllArtifacts().getFiles() method instead. :s4-base:compileJava UP-TO-DATE :s4-base:processResources UP-TO-DATE :s4-base:classes UP-TO-DATE :s4-base:jar UP-TO-DATE :s4-comm:compileJava UP-TO-DATE :s4-comm:processResources UP-TO-DATE :s4-comm:classes UP-TO-DATE :s4-comm:jar UP-TO-DATE :s4-core:compileJava UP-TO-DATE :s4-core:processResources UP-TO-DATE :s4-core:classes UP-TO-DATE :s4-core:compileTestJavaCoreTestUtils.java:5: package org.apache.s4.comm.fixtures does not exist import org.apache.s4.comm.fixtures.CommTestUtils; ^ CoreTestUtils.java:15: cannot find symbol symbol: class CommTestUtils public class CoreTestUtils extends CommTestUtils { ^ CoreTestUtils.java:18: cannot find symbol symbol : method forkProcess(java.lang.String,java.lang.String,java.lang.String) location: class org.apache.s4.fixtures.CoreTestUtils return forkProcess(App.class.getName(), moduleClass.getName(), appClass.getName()); ^ CoreTestUtils.java:22: cannot find symbol symbol : method forkProcess(java.lang.String,java.lang.String[]) location: class org.apache.s4.fixtures.CoreTestUtils return forkProcess(Main.class.getName(), new String[] {}); ^ 4 errors FAILURE: Build failed with an exception. What went wrong: Execution failed for task ':s4-core:compileTestJava'. Cause: Compile failed; see the compiler error output for details. Try: Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. BUILD FAILED Total time: 19.816 secs
          Hide
          kishore gopalakrishna added a comment -

          Karthik,

          I think CommTestUtils are in the test package of comm which means these classes dont make it into s4-comm.jar. So having s4-core depend on s4-comm will not help.

          You need to put the comm-test jar in the dependency of s4-core and then use it in test scope.

          Show
          kishore gopalakrishna added a comment - Karthik, I think CommTestUtils are in the test package of comm which means these classes dont make it into s4-comm.jar. So having s4-core depend on s4-comm will not help. You need to put the comm-test jar in the dependency of s4-core and then use it in test scope.
          Hide
          Karthik Kambatla added a comment -

          The attached patch refactors s4.fixtures across s4-comm and s4-core.

          The corresponding gradle scripts have been updated.

          Note: the AppLoading test case fails and needs attention.

          Show
          Karthik Kambatla added a comment - The attached patch refactors s4.fixtures across s4-comm and s4-core. The corresponding gradle scripts have been updated. Note: the AppLoading test case fails and needs attention.
          Hide
          Matthieu Morel added a comment -

          committed as 111959da9ee348424f3a21540691f206f4517283

          Note: Apploading test is fixed by S4-24 (I will generate a patch later)

          Show
          Matthieu Morel added a comment - committed as 111959da9ee348424f3a21540691f206f4517283 Note: Apploading test is fixed by S4-24 (I will generate a patch later)

            People

            • Assignee:
              Karthik Kambatla
              Reporter:
              Karthik Kambatla
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development