Job Detail "failures" tab test failure functionality

[chadlwilson]

Hi all - anyone have any context of the status of the job detail test failures tab?

Based on #9578, plus the WTF accidentally left behind below:

gocd/server/src/main/java/com/thoughtworks/go/server/presentation/models/JobDetailPresentationModel.java

Lines 157 to 160 in 3e94821

	// TODO: ketanpkr, check this definition
	public boolean hasFailedTests() {
	return hasTests() && false;
	}

and

gocd/server/src/main/webapp/WEB-INF/vm/build_detail/_failures.vm

Lines 17 to 40 in 5516e90

	<div>
	#if (!$presenter.hasFailedTests())
	#if ($presenter.isFailed())
	#parse("build_detail/_build_output_raw.vm")
	#else
	#if ($presenter.hasTests())
	No failures
	#else
	#parse("build_detail/_test_output_config.vm")
	#end
	#end
	#end
	#if ($presenter.hasFailedTests())
	<h2 class="subtitle collapsible_title title_message_expanded">Failed tests</h2>
	<div class="collapsible_content">
	#if ($useIframeSandbox)
	<iframe sandbox="allow-scripts" src="$req.getContextPath()/${presenter.indexPageURL}" width="95%" height="200" frameborder="0"></iframe>
	#else
	<iframe src="$req.getContextPath()/${presenter.indexPageURL}" width="95%" height="200" frameborder="0"></iframe>
	#end
	</div>
	<br/>
	#end

It looks like it was always consider every job to have hasFailedTests = false so the ability to show test failures on this tab appears to have been removed since 19.10 (based on this commit)? Am I missing something - was there some intention to remove this functionality?

The code used to do

    public boolean hasFailedTests() {
        return hasTests() && toInt(getProperties().getValue(TestReportGenerator.FAILED_TEST_COUNT)) > 0;
    }

...however the comment on 35d1f5e seems to imply intention to remove this functionality, citing better tools to do this kind of thing (which personally I generally agree with, in terms of using a tool to generate an HTML report).

In which case, it seems there is still a bit of tidy up to be done in the code, and docs such as https://docs.gocd.org/current/faq/dev_understand_why_build_broken.html which are misleading now.

If so, what should be removed? What is the remaining intent of the "Failures" tab? Currently it looks like it doesn't have much purpose left behind as it appears to

• displays console logs if the job is failed
• displays "No Failures" if job is not failed, but job has tests
• otherwise tells you how to configure test artifacts

[arvindsv]

Yes, I think it was Ketan's intention to remove it completely, but maybe it was derailed in between. Maybe the whole tab should be removed, along with the documentation. Without a plugin-like architecture there, just depending on xUnit reports wasn't working well, with newer test frameworks not adhering to the admittedly outdated format.

[chadlwilson]

Yeah, it's kind of an endless game - and most wrapping build tool ecosystems appear to have decent test report formatters these days; or at least make the command line nice enough to work with (unlike Gradle for Java!).

Was there any conclusion/plan for the Tests tab in general? (which still needs to parse xUnit/JUnit XML even if we get rid of Failures) Since this only looks at the summary data, I imagine it is less brittle.

The main nice thing about tools that "understand" your individual tests is that they can help you highlight individual tests that are consistently flaky over time so you can focus your effort. But desiring this type of functionality has tended to make me feel like a dinosaur, seeming more akin to the type of stuff that things that the "mid generation" of tooling such as GoCD/Bamboo/TeamCity/Jenkins+plugins focused on.

Nevertheless, I will see if I can look at cleaning up the docs (and the Failures tab, at least) in between my other "initiatives" :-) Any clean-up here will make overdue tech debt such as velocity migrations/upgrades/replacement incrementally easier to work with by reducing the number of Velocity template UIs we have.

[arvindsv]

I don't remember much about the "Tests" tab, but I used to see that as very similar to "Failures". So, if one goes, the other might need to / should?

[chadlwilson]

They are related in that they both relied upon parsing JUnit or NUnit format XML reports, i believe from test-designated artifacts, and they both are kind of ugly/underdeveloped now (XSLTed into a fragment displayed via velocity template into an iframe with no style 😬 ).

The summary is not so useful without ability to drill down into at least the failures, but perhaps it's somehow still useful to someone?

I can start by removing the Failures tab now (since it appears completely broken anyway), and announcing the removal of the tests tab for subsequent release or something? I'm not sure if there are additional implications for the artifact type of test as well, since I believe this is the only way to designate a build output folder as containing possible XUnit reports for parsing.

[chadlwilson]

Submitted a PR for removal of Failures tab alone at #9662 and updated the docs already.

If you still think the whole tests tab should go as part of this, I can follow up with that. It still looks ugly, but still kind-of seems to work in terms if displaying the test numbers and some details of the test failures, but I haven't validated that with real JUnit XML output recently, only a pre-canned report where it did display some detail for tests that had error or failure state.

[arvindsv]

I can start by removing the Failures tab now (since it appears completely broken anyway), and announcing the removal of the tests tab for subsequent release or something?

I think this is a good approach. Let's not remove it without an announcement.