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

Clicking the test trend chart opens wrong per-build URL ("test", not "testResult")

      When I click Build #2039 in the "Test Result Trend" chart at <https://SERVER/jenkins/job/REPO/job/master/>, the Web browser navigates to <https://SERVER/jenkins/job/REPO/job/master/2039/test/>, which immediately redirects to <https://SERVER/jenkins/job/REPO/job/master/2039/>. It should instead navigate to the "Test Result" page at <https://SERVER/jenkins/job/REPO/job/master/2039/testReport/>.

      This might be fallout from the JENKINS-62096 ECharts migration. I don't think this is specific to the mstest plugin, because the build.xml file has a <hudson.tasks.junit.TestResultAction plugin="junit@1.48"> element that has neither "test" nor "testReport" in XML content or attributes.

      At <https://SERVER/jenkins/job/REPO/job/master/test/>, the chart correctly links to <https://SERVER/jenkins/job/REPO/job/master/2039/testReport/>. This version of the trend chart however seems to use the deprecated TestResultProjectAction.doTrend method rather than ECharts.

      Related source code:

          [JENKINS-64582] Clicking the test trend chart opens wrong per-build URL ("test", not "testResult")

          If I understand correctly, the problem is that ECharts API Plugin expects the getUrlName() methods of the per-job and per-build actions to return the same string, but the actions of JUnit Plugin return different strings:

          There might be two ways to fix this problem:

          • 1A. Make TestResultProjectAction/floatingBox.jelly pass the "testReport" name of AbstractTestResultAction to ECharts API Plugin as a separate attribute, like <c:trend-chart it="${from}" tool="testReport" …/>. Make ECharts API Plugin use that if specified, instead of getUrlName(). Perhaps that could be done with the ?: operator in JEXL used by Jelly.
          • 1B. Make TestResultProjectAction.getUrlName() return "testReport" rather than "test".
            This would change the URL of the deprecated non-ECharts trend chart.

          There seems to be a type mismatch as well: trend-chart.jelly in echarts-api-plugin declares the "it" attribute as having type AsyncTrendJobAction (source, Javadoc), but TestResultProjectAction/floatingBox.jelly in junit-plugin instead provides TestResultProjectAction, which is not derived from AsyncTrendJobAction. Both implement AsyncTrendChart (source, Javadoc), though. I suppose there are two ways to fix this:

          • 2A. Make trend-chart.jelly declare the "it" parameter as AsyncTrendChart rather than AsyncTrendJobAction.
          • 2B. Make TestResultProjectAction extend AsyncTrendJobAction rather than java.lang.Object. This is what README of echarts-api-plugin recommends anyway.

          drulli, what do you think about the possible ECharts API Plugin changes in 1A and 2A?

          Kalle Niemitalo added a comment - If I understand correctly, the problem is that ECharts API Plugin expects the getUrlName() methods of the per-job and per-build actions to return the same string, but the actions of JUnit Plugin return different strings: In the per-job TestResultProjectAction ( source , Javadoc ), getUrlName() returns "test". URLs like < https://SERVER/jenkins/job/REPO/job/master/test/trend? > have been used for server-side rendering of the chart image but that feature is deprecated. In the per-build AbstractTestResultAction ( source , Javadoc ), getUrlName() returns "testReport". URLs like < https://SERVER/jenkins/job/REPO/job/master/2039/testReport/ > are used for displaying per-build test results. I assume users expect these URLs to keep working. There might be two ways to fix this problem: 1A. Make TestResultProjectAction/floatingBox.jelly pass the "testReport" name of AbstractTestResultAction to ECharts API Plugin as a separate attribute, like <c:trend-chart it="${from}" tool="testReport" …/>. Make ECharts API Plugin use that if specified, instead of getUrlName(). Perhaps that could be done with the ?: operator in JEXL used by Jelly. 1B. Make TestResultProjectAction.getUrlName() return "testReport" rather than "test". This would change the URL of the deprecated non-ECharts trend chart. There seems to be a type mismatch as well: trend-chart.jelly in echarts-api-plugin declares the "it" attribute as having type AsyncTrendJobAction ( source , Javadoc ), but TestResultProjectAction/floatingBox.jelly in junit-plugin instead provides TestResultProjectAction, which is not derived from AsyncTrendJobAction. Both implement AsyncTrendChart ( source , Javadoc ), though. I suppose there are two ways to fix this: 2A. Make trend-chart.jelly declare the "it" parameter as AsyncTrendChart rather than AsyncTrendJobAction. 2B. Make TestResultProjectAction extend AsyncTrendJobAction rather than java.lang.Object. This is what README of echarts-api-plugin recommends anyway. drulli , what do you think about the possible ECharts API Plugin changes in 1A and 2A?

          Ulli Hafner added a comment -

          1A. Make TestResultProjectAction/floatingBox.jelly pass the "testReport" name of AbstractTestResultAction to ECharts API Plugin as a separate attribute, like <c:trend-chart it="${from}" tool="testReport" …/>. Make ECharts API Plugin use that if specified, instead of getUrlName(). Perhaps that could be done with the ?: operator in JEXL used by Jelly.
          1B. Make TestResultProjectAction.getUrlName() return "testReport" rather than "test".
          This would change the URL of the deprecated non-ECharts trend chart.

          I think it would make sense to make to provide an attribute url that replaces the default ${it.urlName} if set (1A).

          2A. Make trend-chart.jelly declare the "it" parameter as AsyncTrendChart rather than AsyncTrendJobAction.
          2B. Make TestResultProjectAction extend AsyncTrendJobAction rather than java.lang.Object. This is what README of echarts-api-plugin recommends anyway.

          I think that I introduced the interface after writing the jelly file since some views cannot use the fixed base class. So using the interface AsyncTrendChart makes more sense (2a).

          Ulli Hafner added a comment - 1A. Make TestResultProjectAction/floatingBox.jelly pass the "testReport" name of AbstractTestResultAction to ECharts API Plugin as a separate attribute, like <c:trend-chart it="${from}" tool="testReport" …/>. Make ECharts API Plugin use that if specified, instead of getUrlName(). Perhaps that could be done with the ?: operator in JEXL used by Jelly. 1B. Make TestResultProjectAction.getUrlName() return "testReport" rather than "test". This would change the URL of the deprecated non-ECharts trend chart. I think it would make sense to make to provide an attribute url that replaces the default ${it.urlName} if set (1A). 2A. Make trend-chart.jelly declare the "it" parameter as AsyncTrendChart rather than AsyncTrendJobAction. 2B. Make TestResultProjectAction extend AsyncTrendJobAction rather than java.lang.Object. This is what README of echarts-api-plugin recommends anyway. I think that I introduced the interface after writing the jelly file since some views cannot use the fixed base class. So using the interface AsyncTrendChart makes more sense (2a).

          AsyncTrendChart has no getUrlName() method, though, so it would seem a bit odd if trend-chart.jelly declared the it attribute as AsyncTrendChart but kept reading ${it.urlName}. Oh well, Stapler jelly tag library reference says the attribute tag is for documentation only.

          Kalle Niemitalo added a comment - AsyncTrendChart has no getUrlName() method, though, so it would seem a bit odd if trend-chart.jelly declared the it attribute as AsyncTrendChart but kept reading ${it.urlName}. Oh well, Stapler jelly tag library reference says the attribute tag is for documentation only.

            Unassigned Unassigned
            kon Kalle Niemitalo
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated: