When the warnings count is successively zero, no y-axis legend is shown on the trend graph.

          [JENKINS-56594] Warnings trend graph has no y-axis legend

          David Aldrich added a comment -

          I suggest a minimum range of 1. So the y-axis would show ticks '0' and '1' as a minimum.

          David Aldrich added a comment - I suggest a minimum range of 1. So the y-axis would show ticks '0' and '1' as a minimum.

          Same if only one issue is found, 0.4 errors does not make sense. The old warnings plugin had only 0 and 1, nothing between.

          Michael Düsterhus added a comment - Same if only one issue is found, 0.4 errors does not make sense. The old warnings plugin had only 0 and 1, nothing between.

          Ulli Hafner added a comment -

          Yes, the new EChart JS lib that I am using seems to provide no integer based axis yet. We need to ask on their mailing list if and how that is possible.

          Ulli Hafner added a comment - Yes, the new EChart JS lib that I am using seems to provide no integer based axis yet. We need to ask on their mailing list if and how that is possible.

          Ulli Hafner added a comment - - edited

          Maybe it would make sense to hide the chart if there are no warnings and show the same screen as in the details:

          Ulli Hafner added a comment - - edited Maybe it would make sense to hide the chart if there are no warnings and show the same screen as in the details:

          I recently reached out to the EChart mailing list to see if it is possible to provide an integer based axis, however, have yet to receive a response. In the meantime, does anyone have any documentation on how the EChart JS lib is being used to display graphs within the plugin or where in the source code the graph is being displayed to the UI? Im rather new and would like to try and create a POC for hiding the chart and showing the no issues screen when there are no warnings so I'll take any help I can get, thanks!

          Colin Wlodkowski added a comment - I recently reached out to the EChart mailing list to see if it is possible to provide an integer based axis, however, have yet to receive a response. In the meantime, does anyone have any documentation on how the EChart JS lib is being used to display graphs within the plugin or where in the source code the graph is being displayed to the UI? Im rather new and would like to try and create a POC for hiding the chart and showing the no issues screen when there are no warnings so I'll take any help I can get, thanks!

          Ulli Hafner added a comment - - edited

          I think it would make sense to move the decision of what to show one layer up and not in the chart code. So the UI should simply have an if-else. This could be done similar to the other UI view: https://github.com/jenkinsci/warnings-ng-plugin/blob/master/src/main/resources/io/jenkins/plugins/analysis/core/model/IssuesDetail/index.jelly#L23.

          The code for the trend is located in https://github.com/jenkinsci/warnings-ng-plugin/blob/master/src/main/resources/io/jenkins/plugins/analysis/core/model/JobAction/floatingBox.jelly. So you need to have the if-else here and show the trend or something else. The if needs to test an boolean property of ${from} which is the instance of the https://github.com/jenkinsci/warnings-ng-plugin/blob/master/src/main/java/io/jenkins/plugins/analysis/core/model/JobAction.java Here we should have a method in the style of https://github.com/jenkinsci/warnings-ng-plugin/blob/master/src/main/java/io/jenkins/plugins/analysis/core/model/JobAction.java#L201 that iterates over all the results and checks if the number of warnings = 0. 

          Ulli Hafner added a comment - - edited I think it would make sense to move the decision of what to show one layer up and not in the chart code. So the UI should simply have an if-else . This could be done similar to the other UI view: https://github.com/jenkinsci/warnings-ng-plugin/blob/master/src/main/resources/io/jenkins/plugins/analysis/core/model/IssuesDetail/index.jelly#L23 . The code for the trend is located in https://github.com/jenkinsci/warnings-ng-plugin/blob/master/src/main/resources/io/jenkins/plugins/analysis/core/model/JobAction/floatingBox.jelly . So you need to have the if-else here and show the trend or something else. The if needs to test an boolean property of ${from } which is the instance of the https://github.com/jenkinsci/warnings-ng-plugin/blob/master/src/main/java/io/jenkins/plugins/analysis/core/model/JobAction.java  Here we should have a method in the style of https://github.com/jenkinsci/warnings-ng-plugin/blob/master/src/main/java/io/jenkins/plugins/analysis/core/model/JobAction.java#L201  that iterates over all the results and checks if the number of warnings = 0. 

          Allan C added a comment -

          Replicated this bug successfully on Jenkins 2.204.2, with a simple freestyle sample (execute shell).

          Allan C added a comment - Replicated this bug successfully on Jenkins 2.204.2, with a simple freestyle sample (execute shell).

          Allan C added a comment - - edited

          drulli Hi Ulli. Hope you are safe and sound. wlodkowc and I have been working on this remotely. We created a boolean method in JobAction class to iterate through all previous results, and check if the number of warnings is 0. Now we are trying to test this new method. 

          We found JobActionITest.java but it does not seem to be a proper test class for us. Do you mind sharing some of your insights? Should we create our own test class, or is there one available that we are not aware of? If we do need to create a new test class, what is the simplest way to create a build result with different numbers of warnings and errors? Thank you and take care.

          Allan C added a comment - - edited drulli Hi Ulli. Hope you are safe and sound. wlodkowc and I have been working on this remotely. We created a boolean method in JobAction class to iterate through all previous results, and check if the number of warnings is 0. Now we are trying to test this new method.  We found JobActionITest.java but it does not seem to be a proper test class for us. Do you mind sharing some of your insights? Should we create our own test class, or is there one available that we are not aware of? If we do need to create a new test class, what is the simplest way to create a build result with different numbers of warnings and errors? Thank you and take care.

          Ulli Hafner added a comment - - edited

          This actually depends a little bit on your implementation. Can't you create a draft PR where we can discuss it directly in the source code?

          I think the JobAction class still is missing a unit test (JENKINS-59619). So without seeing your code I think there are two ways to test the new method: use an integration test that creates some builds with 0 warnings - and another test case that has one build with > 0 warnings. You can reuse any of the integration tests that build a project with warning. Then you can check the boolean property. The other way would be to extract your code into a separate class that has as input a `BuildHistory` and as output a boolean. Then you can use a stub to simulate the build results.

          Ulli Hafner added a comment - - edited This actually depends a little bit on your implementation. Can't you create a draft PR where we can discuss it directly in the source code? I think the JobAction class still is missing a unit test ( JENKINS-59619 ). So without seeing your code I think there are two ways to test the new method: use an integration test that creates some builds with 0 warnings - and another test case that has one build with > 0 warnings. You can reuse any of the integration tests that build a project with warning. Then you can check the boolean property. The other way would be to extract your code into a separate class that has as input a `BuildHistory` and as output a boolean. Then you can use a stub to simulate the build results.

          Allan C added a comment -

          drulli We just created a new pull request: https://github.com/jenkinsci/warnings-ng-plugin/pull/436 Thank you.

          Allan C added a comment - drulli We just created a new pull request:  https://github.com/jenkinsci/warnings-ng-plugin/pull/436  Thank you.

            drulli Ulli Hafner
            davida2009 David Aldrich
            Votes:
            2 Vote for this issue
            Watchers:
            7 Start watching this issue

              Created:
              Updated:
              Resolved: