Add a list of all Skipped tests to the Test Result report #106

nirgilboa · 2018-06-26T07:23:41Z

In a large test suite, it is useful to easily see a list of all skipped tests, as well as their age, i.e. for how many builds they've been skipped.
Previously it was only possible to view all tests in a given class, then sort by result to view the Skipped tests in it.

Note that I've left the "Duration" column, as tests may sometimes be skipped during execution, but it might be better to remove it to reduce clutter - I could use some feedback on that.

nirgilboa · 2018-06-30T19:47:17Z

@rsandell @uhafner I'd appreciate your feedback, thanks 😃

uhafner

Any chance for an integration test? Is there an existing one that could be adapted?

uhafner · 2018-07-05T20:45:04Z

src/main/java/hudson/tasks/junit/CaseResult.java

+     */
+    @Exported(visibility=9)
+    public int getSkippedSince() {
+        // If we haven't calculated failedSince yet, and we should,


skippedSince

uhafner · 2018-07-05T20:47:03Z

src/main/java/hudson/tasks/junit/CaseResult.java

+        if (skippedSince==0 && getSkipCount()==1) {
+            CaseResult prev = getPreviousResult();
+            if(prev!=null && !prev.isPassed())
+                this.skippedSince = prev.getSkippedSince();


What happens if in previous the test failed and therefore it has been disabled now? Then previous.isPassed is false, too. Might be a good setup for an integration test.

You're absolutely right - when a test becomes skipped, its skippedSince counter is set to the current build number.
In a subsequent build:

If the test passes or fails, the skippedSince counter will be reset to 0.

If the test is skipped again, the skippedSince is carried over from the previous build.

uhafner · 2018-07-05T20:48:55Z

src/main/java/hudson/tasks/junit/CaseResult.java

+        // do it now.
+        if (skippedSince==0 && getSkipCount()==1) {
+            CaseResult prev = getPreviousResult();
+            if(prev!=null && !prev.isPassed())


It would make sense to always use {} for ifs.

2. Comments typo, fit on one line 3. if brackets included

2. Avoid NPE possibility in getFailedSince() test

nirgilboa · 2018-07-08T15:02:35Z

Added an integration test which indeed helped to fix a logical error in getSkippedSince().
Thanks for your feedback!!

uhafner

LGTM

src/test/java/hudson/tasks/junit/HistoryTest.java

deepy · 2018-07-23T11:19:02Z

src/main/java/hudson/tasks/junit/CaseResult.java

+     *
+     * If {@link #isPassed() passing}, this field is left unused to 0.
+     */
+    private /*final*/ int skippedSince;


shouldn't /*final*/ be removed?

Thanks - removing

deepy · 2018-07-23T11:19:38Z

src/main/java/hudson/tasks/junit/CaseResult.java

+     * If this test was skipped, then return the build number
+     * when this test was first skipped.
+     */
+    @Exported(visibility=9)


Shouldn't the visibility of the skipped tests be higher than the visibility of failed tests?

On second thought I think the visibility for getFailedSince and getSkippedSince should be identical.

nirgilboa · 2018-07-30T15:10:18Z

@jglick ping

nirgilboa · 2018-09-06T08:30:02Z

@jglick @abayer
Could you please review and decide on this?

src/main/java/hudson/tasks/test/MetaTabulatedResult.java

src/main/resources/lib/hudson/test/aggregated-failed-tests.jelly

olivergondza · 2018-11-28T19:07:52Z

I like the change in general. One small thing I would like to see improved is making the list of skipped tests less significant in the UI. Collapsed by default? Moved to the very bottom? It is useful for sure but probably distracting most of the time...

zbynek · 2019-04-13T20:46:37Z

src/main/java/hudson/tasks/junit/CaseResult.java

+        if (skippedSince==0 && getSkipCount()==1) {
+            CaseResult prev = getPreviousResult();
+            if(prev!=null && prev.isSkipped()){
+                this.skippedSince = prev.getSkippedSince();


Unlike failedSince, skippedSince is not available in XML, so this could recursively load all the skipped builds leading to StackOverflowError like JENKINS-31660 . IMHO it would be safer to save skippedSince in XML too and assume current build is the first skipped one if no information is found in XML, avoiding recursion. (Related to #117.)

Good catch! I will review this and suggest a fix

@zbynek I've given this some thought.. as I understand we only parse the XML and have no control over its contents. Do you see any other way to protect against a StackOverflowError ?

to prevent issues with implementations predating it

nirgilboa · 2019-05-31T13:54:02Z

@olivergondza
Skipped tests table collapsed by default, accessible using a link placed at the bottom of the page.
I've tried to stay consistent with the style on the summary page.
Screenshots below:

Link is hidden and table is shown:

zbynek · 2019-06-01T09:17:28Z

The skippedSince value is saved to XML (automatic saving of all fields, IIUC implemented by XStream2), but in most cases it's not computed before saving. See SuiteResultTest::testSuiteResultPersistence for implementation of saving test. For failedSince the freeze method makes sure that the field is computed before saving. If you add recomputeSkippedSinceIfNeeded() to CaseResult::freeze, saving will work, but it may lead to loading all the previous results from the disk. So recomputeSkippedSinceIfNeeded should fall back to previous build number if previous build failed but we don't know since when it's failing.

Also fixing the freeze method will cause a failure of the test testPreviousBuildNotLoaded from #117. Other way of testing for deep recursion is needed (maybe include 2 builds and make sure that we don't call getPreviousBuild on the older one?)

hodnajit · 2022-07-22T10:20:46Z

Is it possible, that this feature will be merged? :) I would like to use this feature as well :)

SebastianSolidwork · 2023-02-14T14:21:51Z

I'd love to see this feature available. What is lacking for a release?
Basically lack of engagement? :-|

timja · 2023-02-14T15:10:00Z

I'd love to see this feature available. What is lacking for a release? Basically lack of engagement? :-|

reviewing if #106 (comment) needs fixing and then resolving conflicts and testing it end 2 end. Basically needs someone (anyone) to pick it up and get it over the finish line.

nirgilboa · 2023-02-14T15:48:14Z

I'll be happy to revive this and push it if there's demand :)

SebastianSolidwork · 2023-02-15T08:05:06Z

Nice! There is demand :-) How could you have waited for so long !?!?12 ;-)

I would interested in the All Tests the least.
How about making All Skipped Tests as second table, like in the original comment?
At least configurable?
If this is too much for here, I would create another issue.

olivergondza · 2023-02-25T10:50:15Z

@nirgilboa, please resolve the conflicts. Thanks!

SebastianSolidwork · 2023-05-23T06:10:18Z

@nirgilboa Any news on this one?

nirgilboa · 2023-05-23T15:29:30Z

Still here :) Sorry switched into a new role at work

Promise promise promise to complete it soon

Add a list of all Skipped tests to the Test Result report

6da8202

uhafner reviewed Jul 5, 2018

View reviewed changes

nirgilboa added 2 commits July 8, 2018 17:59

1. Logic fix in getSkippedSince()

801310e

2. Comments typo, fit on one line 3. if brackets included

1. Integration test for getSkippedSince()

6aad9ff

2. Avoid NPE possibility in getFailedSince() test

jglick requested review from rsandell and uhafner July 9, 2018 21:18

uhafner approved these changes Jul 10, 2018

View reviewed changes

src/test/java/hudson/tasks/junit/HistoryTest.java Show resolved Hide resolved

deepy reviewed Jul 23, 2018

View reviewed changes

Remove /*final*/ comment

74f043a

olivergondza requested changes Nov 28, 2018

View reviewed changes

src/main/java/hudson/tasks/test/MetaTabulatedResult.java Outdated Show resolved Hide resolved

olivergondza reviewed Nov 28, 2018

View reviewed changes

src/main/resources/lib/hudson/test/aggregated-failed-tests.jelly Outdated Show resolved Hide resolved

zbynek reviewed Apr 13, 2019

View reviewed changes

nirgilboa added 5 commits May 16, 2019 22:50

Revert rename of reusable tag

c8545c6

Method must be non-abstract

c966bff

to prevent issues with implementations predating it

Merge branch 'master' into include-skipped-results

c33e2d0

Add recomputeSkippedSinceIfNeeded for consistency, fix typo

ee98b7e

Skipped tests accessible with a link

45168ac

uhafner mentioned this pull request Feb 14, 2023

Please add an overview of skipped test cases #490

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a list of all Skipped tests to the Test Result report #106

Add a list of all Skipped tests to the Test Result report #106

nirgilboa commented Jun 26, 2018 •

edited

Loading

nirgilboa commented Jun 30, 2018

uhafner left a comment

uhafner Jul 5, 2018

uhafner Jul 5, 2018

nirgilboa Jul 8, 2018

uhafner Jul 5, 2018

nirgilboa commented Jul 8, 2018

uhafner left a comment

deepy Jul 23, 2018

nirgilboa Jul 24, 2018

deepy Jul 23, 2018

nirgilboa Jul 24, 2018

nirgilboa commented Jul 30, 2018

nirgilboa commented Sep 6, 2018

olivergondza commented Nov 28, 2018

zbynek Apr 13, 2019

nirgilboa Apr 14, 2019

nirgilboa May 17, 2019 •

edited

Loading

nirgilboa commented May 31, 2019 •

edited

Loading

zbynek commented Jun 1, 2019

hodnajit commented Jul 22, 2022

SebastianSolidwork commented Feb 14, 2023

timja commented Feb 14, 2023

nirgilboa commented Feb 14, 2023

SebastianSolidwork commented Feb 15, 2023 •

edited

Loading

olivergondza commented Feb 25, 2023

SebastianSolidwork commented May 23, 2023

nirgilboa commented May 23, 2023

Add a list of all Skipped tests to the Test Result report #106

Are you sure you want to change the base?

Add a list of all Skipped tests to the Test Result report #106

Conversation

nirgilboa commented Jun 26, 2018 • edited Loading

nirgilboa commented Jun 30, 2018

uhafner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nirgilboa commented Jul 8, 2018

uhafner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nirgilboa commented Jul 30, 2018

nirgilboa commented Sep 6, 2018

olivergondza commented Nov 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nirgilboa May 17, 2019 • edited Loading

Choose a reason for hiding this comment

nirgilboa commented May 31, 2019 • edited Loading

zbynek commented Jun 1, 2019

hodnajit commented Jul 22, 2022

SebastianSolidwork commented Feb 14, 2023

timja commented Feb 14, 2023

nirgilboa commented Feb 14, 2023

SebastianSolidwork commented Feb 15, 2023 • edited Loading

olivergondza commented Feb 25, 2023

SebastianSolidwork commented May 23, 2023

nirgilboa commented May 23, 2023

nirgilboa commented Jun 26, 2018 •

edited

Loading

nirgilboa May 17, 2019 •

edited

Loading

nirgilboa commented May 31, 2019 •

edited

Loading

SebastianSolidwork commented Feb 15, 2023 •

edited

Loading