Increase automated test coverage

      Automated test coverage of the version column plugin is lower that is desired. Improve the automated test coverage by submitting pull requests with new automated tests.

      Review current coverage

      Review the current test coverage with the commands:

      Linux

      $ mvn -P enable-jacoco clean install jacoco:report
      $ xdg-open target/site/jacoco/index.html
      

      Windows

      C:\Users\Yourname > mvn -P enable-jacoco clean install jacoco:report
      C:\Users\Yourname > start target\site\jacoco\index.html
      

      Create a new test for a class that is not well covered

      Most integrated development environments have tools that will create a test stub that is a good beginning. Apache Netbeans has "Create / Update tests" . JetBrains IntelliJ has "Create tests". Visual Studio Code has the "Extension Pack for Java".

      Use the IDE or your own coding to create a test for one of the classes that is not well covered by tests.

          [JENKINS-70560] Improve version column plugin test coverage

          Shreya added a comment -

          markewaite 
          As you suggested, have created a Jira ticket for this issue. Wanted to know that should I make a separate PR for different methods that I cover, or should I work on a single WIP PR?

          I have labelled the PR as Work in Progress right now

          Shreya added a comment - markewaite   As you suggested, have created a Jira ticket for this issue. Wanted to know that should I make a separate PR for different methods that I cover, or should I work on a single WIP PR? I have labelled the PR as Work in Progress right now

          Mark Waite added a comment -

          yashre_bh a single Jira issue is enough to cover many pull requests.

          Mark Waite added a comment - yashre_bh a single Jira issue is enough to cover many pull requests.

          Arnab added a comment - - edited

          Hello markewaite, I was looking into the readResolve() function of JVMVersionMonitor and it had a check - 

          line 75
           public Object readResolve() {
                  if (disconnect != null)

          {              this.setIgnored(!disconnect);                   }

                  return this;
          }

          but the disconnect variable is private - (line 53)
          private transient Boolean disconnect;
           
          This disconnect variable is not getting assigned anywhere in this Class (can't be assigned elsewhere too, since it is private). I also did a search on the repo - https://github.com/search?q=repo%3Ajenkinsci%2Fversioncolumn-plugin%20disconnect&type=code
           
          Conclusion - It appears to me that this is somewhat redundant and is not serving it's purpose (if any). So, maybe we should remove it or modify it if necessary.
           
          Would like your thoughts on this. Thank You. P.S. - I also checked by adding another function to print the value of disconnect variable after operations like setDisconnect() and it is always null.

          Arnab added a comment - - edited Hello markewaite , I was looking into the readResolve() function of JVMVersionMonitor and it had a check -  line 75  public Object readResolve() {         if (disconnect != null) {              this.setIgnored(!disconnect);                   }         return this; } but the disconnect variable is private - (line 53) private transient Boolean disconnect;   This disconnect variable is not getting assigned anywhere in this Class (can't be assigned elsewhere too, since it is private). I also did a search on the repo - https://github.com/search?q=repo%3Ajenkinsci%2Fversioncolumn-plugin%20disconnect&type=code   Conclusion - It appears to me that this is somewhat redundant and is not serving it's purpose (if any). So, maybe we should remove it or modify it if necessary.   Would like your thoughts on this. Thank You. P.S. - I also checked by adding another function to print the value of disconnect variable after operations like setDisconnect() and it is always null.

          Mark Waite added a comment -

          Thanks for your interest in that private field. I'm not willing to remove it or have it removed by someone else.

          I have very limited experience with readResolve() methods and am generally unwilling to change them without exhaustive testing for compatibility with previous versions of the plugin. I'm not willing to invest that level of testing into the Versions Node Monitors plugin.

          If you'd like to learn more about readResolve() in Jenkins plugins, refer to the persistent objects section of the developer guide. Pay particular attention to backward compatibility with XStream . You may also want to review the "compatibility matters" section of the governance document.

          Mark Waite added a comment - Thanks for your interest in that private field. I'm not willing to remove it or have it removed by someone else. I have very limited experience with readResolve() methods and am generally unwilling to change them without exhaustive testing for compatibility with previous versions of the plugin. I'm not willing to invest that level of testing into the Versions Node Monitors plugin . If you'd like to learn more about readResolve() in Jenkins plugins, refer to the persistent objects section of the developer guide . Pay particular attention to backward compatibility with XStream . You may also want to review the "compatibility matters" section of the governance document.

          Arnab added a comment -

          Ah! I get it. Though it's use was not evident at the first glance but it is important when when we don't get the persistent data while loading from XML or if you want to put any "non-trivial default value" same goes for that private field XStream will set the value but by adding transient it won't be written back to XML.

          Thanks for sharing this

          Arnab added a comment - Ah! I get it. Though it's use was not evident at the first glance but it is important when when we don't get the persistent data while loading from XML or if you want to put any "non-trivial default value" same goes for that private field XStream will set the value but by adding transient it won't be written back to XML. Thanks for sharing this

          Arnab added a comment -

          Hello markewaite, just sent a PR for improved test coverage of "DescriptorImpl" (86% -> 100%).

          Arnab added a comment - Hello markewaite , just sent a PR for improved test coverage of "DescriptorImpl" (86% -> 100%).

            orerpa Orlando
            yashre_bh Shreya
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated: