Details

    • Type: Improvement
    • Status: Closed
    • Priority: Trivial
    • Resolution: Implemented
    • Affects Version/s: 1.3.0
    • Fix Version/s: 1.3.0
    • Component/s: Tests
    • Labels:
      None

      Description

      The following test base classes don't contain any test methods and should be marked as abstract:

      BinaryOperatorTestBase
      DriverTestBase
      UnaryOperatorTestBase

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user zentol opened a pull request:

          https://github.com/apache/flink/pull/3790

          FLINK-6395 [tests] Mark test bases as abstract

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/zentol/flink 6395_testbase_abstract

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/flink/pull/3790.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #3790


          commit 8ae5ad9f9119e2664af248deb304b60991ceee27
          Author: zentol <chesnay@apache.org>
          Date: 2017-04-27T13:59:12Z

          FLINK-6395 [tests] Mark test bases as abstract


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user zentol opened a pull request: https://github.com/apache/flink/pull/3790 FLINK-6395 [tests] Mark test bases as abstract You can merge this pull request into a Git repository by running: $ git pull https://github.com/zentol/flink 6395_testbase_abstract Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3790.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #3790 commit 8ae5ad9f9119e2664af248deb304b60991ceee27 Author: zentol <chesnay@apache.org> Date: 2017-04-27T13:59:12Z FLINK-6395 [tests] Mark test bases as abstract
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tillrohrmann commented on the issue:

          https://github.com/apache/flink/pull/3790

          Not sure whether marking classes without abstract methods abstract makes sense. If we want to prevent instantiations of these classes we could make the constructor also protected.

          Show
          githubbot ASF GitHub Bot added a comment - Github user tillrohrmann commented on the issue: https://github.com/apache/flink/pull/3790 Not sure whether marking classes without abstract methods abstract makes sense. If we want to prevent instantiations of these classes we could make the constructor also protected.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on the issue:

          https://github.com/apache/flink/pull/3790

          As you can see in the `UnaryOperatorTestBase` the constructor is already protected. The thing is that junit complains since the classes contain junit annotations (`@After`, `@Parameterized`) class but can't be instantiated. When i run these test in IntelliJ they are marked as failed tests due to an initialization error.

          Making the classes abstract makes it clear they aren't actually self-contained test classes.

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on the issue: https://github.com/apache/flink/pull/3790 As you can see in the `UnaryOperatorTestBase` the constructor is already protected. The thing is that junit complains since the classes contain junit annotations (`@After`, `@Parameterized`) class but can't be instantiated. When i run these test in IntelliJ they are marked as failed tests due to an initialization error. Making the classes abstract makes it clear they aren't actually self-contained test classes.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tillrohrmann commented on the issue:

          https://github.com/apache/flink/pull/3790

          Alright, now I understand. +1 for merging.

          Show
          githubbot ASF GitHub Bot added a comment - Github user tillrohrmann commented on the issue: https://github.com/apache/flink/pull/3790 Alright, now I understand. +1 for merging.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/flink/pull/3790

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/3790
          Hide
          greghogan Greg Hogan added a comment -

          Implemented in d27d3dda4e850b90267e4b426a3e4993bd0cbac8

          Show
          greghogan Greg Hogan added a comment - Implemented in d27d3dda4e850b90267e4b426a3e4993bd0cbac8

            People

            • Assignee:
              Zentol Chesnay Schepler
              Reporter:
              Zentol Chesnay Schepler
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development