From 21e5ee655135b289bc2e6840be0b0c594053c0be Mon Sep 17 00:00:00 2001
From: Martijn van Groningen <martijn.v.groningen@gmail.com>
Date: Mon, 12 Feb 2018 11:28:40 +0100
Subject: [PATCH] [TEST] Changed how stash dumps are logged in yaml tests in
 case of failures

Currently if a yaml test has a teardown and a test is failing then
a stash dump of a request in the teardown is logged instead of
a stash dump of a request in the test itself.

By handling the logging of stash dumps separately for setup, tests and
teardown yaml sections we shouldn't miss the stash dump of request/response
that is actually causing the yaml test to fail.
---
 .../rest/yaml/ESClientYamlSuiteTestCase.java  | 38 ++++++++++++-------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java
index 5ee78c6942d..f3e98f919d4 100644
--- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java
+++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java
@@ -21,7 +21,6 @@ package org.elasticsearch.test.rest.yaml;
 
 import com.carrotsearch.randomizedtesting.RandomizedTest;
 import org.apache.http.HttpHost;
-import org.apache.lucene.util.IOUtils;
 import org.elasticsearch.Version;
 import org.elasticsearch.client.Response;
 import org.elasticsearch.client.ResponseException;
@@ -144,15 +143,6 @@ public abstract class ESClientYamlSuiteTestCase extends ESRestTestCase {
         return new ClientYamlTestClient(restSpec, restClient, hosts, esVersion);
     }
 
-    @Override
-    protected void afterIfFailed(List<Throwable> errors) {
-        // Dump the stash on failure. Instead of dumping it in true json we escape `\n`s so stack traces are easier to read
-        logger.info("Stash dump on failure [{}]",
-                Strings.toString(restTestExecutionContext.stash(), true, true)
-                        .replace("\\n", "\n").replace("\\r", "\r").replace("\\t", "\t"));
-        super.afterIfFailed(errors);
-    }
-
     public static Iterable<Object[]> createParameters() throws Exception {
         String[] paths = resolvePathsProperty(REST_TESTS_SUITE, ""); // default to all tests under the test root
         List<Object[]> tests = new ArrayList<>();
@@ -318,7 +308,14 @@ public abstract class ESClientYamlSuiteTestCase extends ESRestTestCase {
         if (!testCandidate.getSetupSection().isEmpty()) {
             logger.debug("start setup test [{}]", testCandidate.getTestPath());
             for (DoSection doSection : testCandidate.getSetupSection().getDoSections()) {
-                executeSection(doSection);
+                try {
+                    executeSection(doSection);
+                } catch (AssertionError | Exception e) {
+                    logger.info("Stash dump on setup failure [{}]",
+                            Strings.toString(restTestExecutionContext.stash(), true, true)
+                                    .replace("\\n", "\n").replace("\\r", "\r").replace("\\t", "\t"));
+                    throw e;
+                }
             }
             logger.debug("end setup test [{}]", testCandidate.getTestPath());
         }
@@ -327,12 +324,27 @@ public abstract class ESClientYamlSuiteTestCase extends ESRestTestCase {
 
         try {
             for (ExecutableSection executableSection : testCandidate.getTestSection().getExecutableSections()) {
-                executeSection(executableSection);
+                try {
+                    executeSection(executableSection);
+                } catch (AssertionError | Exception e) {
+                    // Dump the stash on failure. Instead of dumping it in true json we escape `\n`s so stack traces are easier to read
+                    logger.info("Stash dump on test failure [{}]",
+                            Strings.toString(restTestExecutionContext.stash(), true, true)
+                                    .replace("\\n", "\n").replace("\\r", "\r").replace("\\t", "\t"));
+                    throw e;
+                }
             }
         } finally {
             logger.debug("start teardown test [{}]", testCandidate.getTestPath());
             for (DoSection doSection : testCandidate.getTeardownSection().getDoSections()) {
-                executeSection(doSection);
+                try {
+                    executeSection(doSection);
+                } catch (AssertionError | Exception e) {
+                    logger.info("Stash dump on tear down failure [{}]",
+                            Strings.toString(restTestExecutionContext.stash(), true, true)
+                                    .replace("\\n", "\n").replace("\\r", "\r").replace("\\t", "\t"));
+                    // Don't rethrow exception here, that could hide the exception caused by a test.
+                }
             }
             logger.debug("end teardown test [{}]", testCandidate.getTestPath());
         }