Details

    • Sub-task
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • 0.10.1
    • 0.11.0
    • None
    • None

    Attachments

      Activity

        githubbot ASF GitHub Bot added a comment -

        Reamer opened a new pull request, #4559:
        URL: https://github.com/apache/zeppelin/pull/4559

            1. What is this PR for?
              This pull request migrates the JUnit 4 tests to Junit 5 tests.
            1. What type of PR is it?
              Refactoring
            1. What is the Jira issue?
              https://issues.apache.org/jira/browse/ZEPPELIN-5879
            1. How should this be tested?
        • CI
            1. Questions:
        • Does the license files need to update? No
        • Is there breaking changes for older versions? No
        • Does this needs documentation? No
        githubbot ASF GitHub Bot added a comment - Reamer opened a new pull request, #4559: URL: https://github.com/apache/zeppelin/pull/4559 What is this PR for? This pull request migrates the JUnit 4 tests to Junit 5 tests. What type of PR is it? Refactoring What is the Jira issue? https://issues.apache.org/jira/browse/ZEPPELIN-5879 How should this be tested? CI Questions: Does the license files need to update? No Is there breaking changes for older versions? No Does this needs documentation? No
        githubbot ASF GitHub Bot added a comment -

        huage1994 commented on PR #4559:
        URL: https://github.com/apache/zeppelin/pull/4559#issuecomment-1449293326

        Hi @Reamer , this PR is a great job. It all looks good to me.
        I just have a very small question about this: I noticed that you removed the `public` modifier from the `@Test` method but retain the `public` modifier for methods that are decorated by `@BeforEeach` .

        Is there any special purpose in this? I've tried to remove `public` modifier for `@BeforEeach` methods , it looks ok.

        githubbot ASF GitHub Bot added a comment - huage1994 commented on PR #4559: URL: https://github.com/apache/zeppelin/pull/4559#issuecomment-1449293326 Hi @Reamer , this PR is a great job. It all looks good to me. I just have a very small question about this: I noticed that you removed the `public` modifier from the `@Test` method but retain the `public` modifier for methods that are decorated by `@BeforEeach` . Is there any special purpose in this? I've tried to remove `public` modifier for `@BeforEeach` methods , it looks ok.
        githubbot ASF GitHub Bot added a comment -

        Reamer commented on PR #4559:
        URL: https://github.com/apache/zeppelin/pull/4559#issuecomment-1449891496

        > I just have a very small question about this: I noticed that you removed the `public` modifier from the `@Test` method but retain the `public` modifier for methods that are decorated by `@BeforEeach` .
        >
        > Is there any special purpose in this?

        There is no deeper meaning. My IDE runs the sonarlint plugin, this plugin gives me a little hint to remove the public modifier.
        https://rules.sonarsource.com/java/RSPEC-5786

        Unfortunately the hint is missing for @BeforeEach and other methods, so I overlooked deleting the modifier at this point.

        I will go back through the test classes and remove the visibility.

        JUnit5 recommends removing the public modifier.
        https://junit.org/junit5/docs/current/user-guide/#writing-tests-classes-and-methods

        githubbot ASF GitHub Bot added a comment - Reamer commented on PR #4559: URL: https://github.com/apache/zeppelin/pull/4559#issuecomment-1449891496 > I just have a very small question about this: I noticed that you removed the `public` modifier from the `@Test` method but retain the `public` modifier for methods that are decorated by `@BeforEeach` . > > Is there any special purpose in this? There is no deeper meaning. My IDE runs the sonarlint plugin, this plugin gives me a little hint to remove the public modifier. https://rules.sonarsource.com/java/RSPEC-5786 Unfortunately the hint is missing for @BeforeEach and other methods, so I overlooked deleting the modifier at this point. I will go back through the test classes and remove the visibility. JUnit5 recommends removing the public modifier. https://junit.org/junit5/docs/current/user-guide/#writing-tests-classes-and-methods
        githubbot ASF GitHub Bot added a comment -

        huage1994 commented on code in PR #4559:
        URL: https://github.com/apache/zeppelin/pull/4559#discussion_r1122608228

        ##########
        zeppelin-plugins/notebookrepo/s3/pom.xml:
        ##########
        @@ -109,15 +131,11 @@
        <groupId>javax.xml.bind</groupId>
        <artifactId>jaxb-api</artifactId>
        </exclusion>

        • <exclusion>
          + <exclusion>
          <groupId>com.fasterxml.jackson.core</groupId>
          <artifactId>jackson-core</artifactId>
          </exclusion>
        • <exclusion>
        • <groupId>com.fasterxml.jackson.dataformat</groupId>
        • <artifactId>jackson-dataformat-xml</artifactId>
        • </exclusion>
        • <exclusion>
          + <exclusion>

        Review Comment:
        Everything is good except the indentation is misaligned in three places

        ##########
        zeppelin-plugins/notebookrepo/s3/pom.xml:
        ##########
        @@ -129,14 +147,22 @@
        <groupId>com.fasterxml.jackson.core</groupId>
        <artifactId>jackson-databind</artifactId>
        </exclusion>

        • <exclusion>
          + <exclusion>

        Review Comment:
        ```suggestion
        <exclusion>
        ```

        ##########
        zeppelin-plugins/notebookrepo/s3/pom.xml:
        ##########
        @@ -109,15 +131,11 @@
        <groupId>javax.xml.bind</groupId>
        <artifactId>jaxb-api</artifactId>
        </exclusion>

        • <exclusion>
          + <exclusion>

        Review Comment:
        ```suggestion
        <exclusion>
        ```

        ##########
        zeppelin-plugins/notebookrepo/s3/pom.xml:
        ##########
        @@ -109,15 +131,11 @@
        <groupId>javax.xml.bind</groupId>
        <artifactId>jaxb-api</artifactId>
        </exclusion>

        • <exclusion>
          + <exclusion>
          <groupId>com.fasterxml.jackson.core</groupId>
          <artifactId>jackson-core</artifactId>
          </exclusion>
        • <exclusion>
        • <groupId>com.fasterxml.jackson.dataformat</groupId>
        • <artifactId>jackson-dataformat-xml</artifactId>
        • </exclusion>
        • <exclusion>
          + <exclusion>

        Review Comment:
        ```suggestion
        <exclusion>
        ```

        githubbot ASF GitHub Bot added a comment - huage1994 commented on code in PR #4559: URL: https://github.com/apache/zeppelin/pull/4559#discussion_r1122608228 ########## zeppelin-plugins/notebookrepo/s3/pom.xml: ########## @@ -109,15 +131,11 @@ <groupId>javax.xml.bind</groupId> <artifactId>jaxb-api</artifactId> </exclusion> <exclusion> + <exclusion> <groupId>com.fasterxml.jackson.core</groupId> <artifactId>jackson-core</artifactId> </exclusion> <exclusion> <groupId>com.fasterxml.jackson.dataformat</groupId> <artifactId>jackson-dataformat-xml</artifactId> </exclusion> <exclusion> + <exclusion> Review Comment: Everything is good except the indentation is misaligned in three places ########## zeppelin-plugins/notebookrepo/s3/pom.xml: ########## @@ -129,14 +147,22 @@ <groupId>com.fasterxml.jackson.core</groupId> <artifactId>jackson-databind</artifactId> </exclusion> <exclusion> + <exclusion> Review Comment: ```suggestion <exclusion> ``` ########## zeppelin-plugins/notebookrepo/s3/pom.xml: ########## @@ -109,15 +131,11 @@ <groupId>javax.xml.bind</groupId> <artifactId>jaxb-api</artifactId> </exclusion> <exclusion> + <exclusion> Review Comment: ```suggestion <exclusion> ``` ########## zeppelin-plugins/notebookrepo/s3/pom.xml: ########## @@ -109,15 +131,11 @@ <groupId>javax.xml.bind</groupId> <artifactId>jaxb-api</artifactId> </exclusion> <exclusion> + <exclusion> <groupId>com.fasterxml.jackson.core</groupId> <artifactId>jackson-core</artifactId> </exclusion> <exclusion> <groupId>com.fasterxml.jackson.dataformat</groupId> <artifactId>jackson-dataformat-xml</artifactId> </exclusion> <exclusion> + <exclusion> Review Comment: ```suggestion <exclusion> ```
        githubbot ASF GitHub Bot added a comment -

        Reamer merged PR #4559:
        URL: https://github.com/apache/zeppelin/pull/4559

        githubbot ASF GitHub Bot added a comment - Reamer merged PR #4559: URL: https://github.com/apache/zeppelin/pull/4559
        Reamer Philipp Dallig added a comment - Closed via https://github.com/apache/zeppelin/pull/4559

        People

          Reamer Philipp Dallig
          Reamer Philipp Dallig
          Votes:
          0 Vote for this issue
          Watchers:
          Start watching this issue

          Dates

            Created:
            Updated:
            Resolved:

            Slack