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

[david.aldrich@emea.nec.com (JIRA)]

David Aldrich created an issue

Jenkins / JENKINS-56594 Warnings trend graph has no y-axis legend Issue Type: Bug Assignee: Ulli Hafner Attachments: image-2019-03-18-09-41-33-293.png Components: warnings-ng-plugin Created: 2019-03-18 09:41 Environment: Warnings NG plugin 4.0.0 Jenkins 2.164.1 Priority: Minor Reporter: David Aldrich When the warnings count is successively zero, no y-axis legend is shown on the trend graph. Add Comment

This message was sent by Atlassian Jira (v7.11.2#711002-sha1:fdc329d)

[ullrich.hafner@gmail.com (JIRA)]

Ulli Hafner commented on JENKINS-56594

Re: Warnings trend graph has no y-axis legend What is your expected visualization? An arbitrary range 1-10, or not chart at all?

Add Comment

[david.aldrich@emea.nec.com (JIRA)]

David Aldrich commented on JENKINS-56594

Re: Warnings trend graph has no y-axis legend

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

Add Comment

[reitzmichnicht@gmx.de (JIRA)]

Michael D updated an issue

Jenkins / JENKINS-56594

Warnings trend graph has no y-axis legend

Change By: Michael D Attachment: warnings-ng-y-scale.png

Add Comment

[reitzmichnicht@gmx.de (JIRA)]

Michael D commented on JENKINS-56594

Re: Warnings trend graph has no y-axis legend 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.

Add Comment

[ullrich.hafner@gmail.com (JIRA)]

Ulli Hafner commented on JENKINS-56594

Re: Warnings trend graph has no y-axis legend

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.

Add Comment

[ullrich.hafner@gmail.com (JIRA)]

Ulli Hafner updated an issue

Jenkins / JENKINS-56594

Warnings trend graph has no y-axis legend

Change By: Ulli Hafner Labels: newbie-friendly

Add Comment

This message was sent by Atlassian Jira (v7.13.6#713006-sha1:cc4451f)

[ullrich.hafner@gmail.com (JIRA)]

Ulli Hafner updated an issue

Jenkins / JENKINS-56594 Warnings trend graph has no y-axis legend

Change By: Ulli Hafner Attachment: Bildschirmfoto 2019-10-01 um 11.15.53.png

Add Comment

[ullrich.hafner@gmail.com (JIRA)]

Ulli Hafner commented on JENKINS-56594

Re: Warnings trend graph has no y-axis legend Maybe it would make sense to hide the chart if there are no warnings and show the same screen as in the details:

Add Comment

[ullrich.hafner@gmail.com (JIRA)]

Ulli Hafner edited a comment on JENKINS-56594

Re: Warnings trend graph has no y-axis legend

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

!Bildschirmfoto 2019-10-01 um 11.15.53.png|thumbnail!

Add Comment

[wlodkowc@dickinson.edu (JIRA)]

Colin Wlodkowski assigned an issue to Colin Wlodkowski

Jenkins / JENKINS-56594

Warnings trend graph has no y-axis legend

Change By: Colin Wlodkowski Assignee: Ulli Hafner Colin Wlodkowski

Add Comment

[wlodkowc@dickinson.edu (JIRA)]

Colin Wlodkowski commented on JENKINS-56594

Re: Warnings trend graph has no y-axis legend 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!

Add Comment

[ullrich.hafner@gmail.com (JIRA)]

Ulli Hafner commented on JENKINS-56594

Re: Warnings trend graph has no y-axis legend

I think it would make sense to show a different screen should be 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.

Add Comment

[ullrich.hafner@gmail.com (JIRA)]

Ulli Hafner edited a comment on JENKINS-56594

Re: Warnings trend graph has no y-axis legend

I think it would make sense to move the decision of what to show a different screen should be 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.

Add Comment

[ullrich.hafner@gmail.com (JIRA)]

Ulli Hafner edited a comment on JENKINS-56594

Re: Warnings trend graph has no y-axis legend

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.

Add Comment

[ullrich.hafner@gmail.com (JIRA)]

Ulli Hafner edited a comment on JENKINS-56594

Re: Warnings trend graph has no y-axis legend

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.

Add Comment

[chig@dickinson.edu (JIRA)]

Allan C updated an issue

Jenkins / JENKINS-56594

Warnings trend graph has no y-axis legend

Change By: Allan C Attachment: image-2020-02-21-14-12-22-279.png

Add Comment

[chig@dickinson.edu (JIRA)]

Allan C commented on JENKINS-56594

Re: Warnings trend graph has no y-axis legend Replicated this bug successfully on Jenkins 2.204.2, with a simple freestyle sample (execute shell).

Add Comment

[chig@dickinson.edu (JIRA)]

Allan C commented on JENKINS-56594

Re: Warnings trend graph has no y-axis legend

Ulli Hafner Hi Ulli. Hope you are safe and sound. Colin Wlodkowski and I haven't 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.

Add Comment

This message was sent by Atlassian Jira (v7.13.12#713012-sha1:6e07c38)

[ullrich.hafner@gmail.com (JIRA)]

Ulli Hafner commented on JENKINS-56594

Re: Warnings trend graph has no y-axis legend

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.

Add Comment

[ullrich.hafner@gmail.com (JIRA)]

Ulli Hafner edited a comment on JENKINS-56594

[chig@dickinson.edu (JIRA)]

Allan C commented on JENKINS-56594

Re: Warnings trend graph has no y-axis legend

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

Add Comment

[chig@dickinson.edu (JIRA)]

Allan C edited a comment on JENKINS-56594

Re: Warnings trend graph has no y-axis legend

[~drulli] Hi Ulli. Hope you are safe and sound. [~wlodkowc] and I haven't 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.

Add Comment