Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-60940

Convert git client plugin tests from JUnit 3 to JUnit 4

    • 3.13.0

      The git client plugin tests have started their transition process to retire the JUnit 3 based GitAPITestCase, CliGitAPIImplTest, and JGitAPIImplTest. They should be replaced by the parameterized GitClientTest.

      For each test in GitAPITestCase, CliGitAPIImplTest, and JGitAPIImplTest:

      • If it is already tested in GitClientTest, delete the test from the JUnit 3 based tests
      • If it is not already tested in GitClientTest, write a test in GitClientTest or in a new test class and delete the test from the JUnit 3 based tests

      This issue could be worked by multiple people concurrently, so long as they coordinate with one another on the specific tests they are converting.

          [JENKINS-60940] Convert git client plugin tests from JUnit 3 to JUnit 4

          Ayan added a comment -

          Thanks markewaite I'm right now going through the classes as mentioned in the previous discussion, could suggest anything for a beginner?

          Ayan added a comment - Thanks markewaite  I'm right now going through the classes as mentioned in the previous discussion, could suggest anything for a beginner?

          Mark Waite added a comment -

          ayang17 choose any of the methods in GitAPITestCase that are public void test_.*. Fewer statements in the test method will make that method easier to convert to JUnit 4. For example, you might consider:

          • test_getRemoteURL
          • test_empty_comment
          • test_create_branch
          • test_delete_branch
          • test_delete_tag
          • test_list_tags_with_filter
            test_list_tags_without_filter

          Mark Waite added a comment - ayang17 choose any of the methods in GitAPITestCase that are public void test_.* . Fewer statements in the test method will make that method easier to convert to JUnit 4. For example, you might consider: test_getRemoteURL test_empty_comment test_create_branch test_delete_branch test_delete_tag test_list_tags_with_filter test_list_tags_without_filter

          Ayan added a comment -

          Sure, thanks markewaite

          Ayan added a comment - Sure, thanks markewaite

          Ayan added a comment -

          markewaite I implemeted the test_getRemoteURL, wanted your review about the same that my approach is okay or there is some area where I can improve, so I have created a PR, please review it.

          Ayan added a comment - markewaite  I implemeted the test_getRemoteURL, wanted your review about the same that my approach is okay or there is some area where I can improve, so I have created a PR, please review it.

          Mark Waite added a comment -

          Special thanks to ayang17 for the great progress made. Remains open because there are still tests to be converted. The progress is very much appreciated!

          Mark Waite added a comment - Special thanks to ayang17 for the great progress made. Remains open because there are still tests to be converted. The progress is very much appreciated!

          Have gone through the test methods in GitAPITestCase that are to be transformed into JUnit 4 and noticed some of them can not be migrated to GitClientTest because they consume some abstract methods and new host test class is not abstract. Thoughtfully I see two approaches to solve the issue. 1. We can have GitClientTest converted into an abstract class but this comes with a cost for those classes that extends the class. 2. We can create a new test class to handle these test methods. 

          markewaite let me know what you think?

          Daud Kakumirizi added a comment - Have gone through the test methods in GitAPITestCase that are to be transformed into JUnit 4 and noticed some of them can not be migrated to GitClientTest because they consume some abstract methods and new host test class is not abstract. Thoughtfully I see two approaches to solve the issue. 1. We can have GitClientTest converted into an abstract class but this comes with a cost for those classes that extends the class. 2. We can create a new test class to handle these test methods.  markewaite  let me know what you think?

          Mark Waite added a comment -

          I see two approaches to solve the issue. 1. We can have GitClientTest converted into an abstract class but this comes with a cost for those classes that extends the class. 2. We can create a new test class to handle these test methods.

          I prefer a new test class. Large test classes tend to become unwieldy.

          Mark Waite added a comment - I see two approaches to solve the issue. 1. We can have GitClientTest converted into an abstract class but this comes with a cost for those classes that extends the class. 2. We can create a new test class to handle these test methods. I prefer a new test class. Large test classes tend to become unwieldy.

          Daud Kakumirizi added a comment - PR:  https://github.com/jenkinsci/git-client-plugin/pull/814

          Hrushi20 added a comment - - edited

          markewaite, When I try to migrate the GitAPITestCase to junit 4, it is an abstract class. I feel it's better to update the GitAPITestCase to junit 4 in the same class rather than move the tests to GitClientTest as GitClientTest has grown very big and can be difficult to maintain. We can update the internal behaviour of GitAPITestCase to Junit 4 and test it using the extended class i.e(JGitApacheAPIImplTest,CliGitAPIImplTest). What do you think about this?

          Thank You.

           

          Hrushi20 added a comment - - edited markewaite , When I try to migrate the GitAPITestCase to junit 4, it is an abstract class. I feel it's better to update the GitAPITestCase to junit 4 in the same class rather than move the tests to GitClientTest as GitClientTest has grown very big and can be difficult to maintain. We can update the internal behaviour of GitAPITestCase to Junit 4 and test it using the extended class i.e(JGitApacheAPIImplTest,CliGitAPIImplTest). What do you think about this? Thank You.  

          Mark Waite added a comment -

          hrushi20 I believe that the technique you're describing was attempted by kdaud. I would advise against the technique because it is attempting to make a single very large change to that file of tests. I've generally had better results by making a series of small changes. Each of the small changes helps me learn, understand, and better prepare for the later steps.

          If there are too many tests in GitClientTest, then the converted tests can be placed in other classes. The transition is not limited to converting tests from the existing class to a single destination class. They can be converted from their existing single class into many different classes, grouped by the capabilities being tested or by other useful attributes.

          Mark Waite added a comment - hrushi20 I believe that the technique you're describing was attempted by kdaud . I would advise against the technique because it is attempting to make a single very large change to that file of tests. I've generally had better results by making a series of small changes. Each of the small changes helps me learn, understand, and better prepare for the later steps. If there are too many tests in GitClientTest, then the converted tests can be placed in other classes. The transition is not limited to converting tests from the existing class to a single destination class. They can be converted from their existing single class into many different classes, grouped by the capabilities being tested or by other useful attributes.

            hrushi20 Hrushi20
            markewaite Mark Waite
            Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

              Created:
              Updated:
              Resolved: