This is an automated email from the ASF dual-hosted git repository. michaelsmith pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit 24581704d8820dc691a43d69629dce7fac848597 Author: Riza Suminto <[email protected]> AuthorDate: Tue Nov 5 07:38:14 2024 -0800 IMPALA-13512: Print .test file name if PlannerTest fail This patch improve PlannerTest by printing the path to .test file that is failed. It also skip printing VERBOSE plan if PlannerTestOption.EXTENDED_EXPLAIN is specified, since EXTENDED level already contains sufficient details including tuples, sizes, and cardinality. This patch also change target path to save the updated end-to-end test file if --update_results parameter is set. For example: Before: $EE_TEST_LOGS_DIR/tpcds-decimal_v2-q98.test After: $EE_TEST_LOGS_DIR/impala_updated_results/tpcds/queries/tpcds-decimal_v2-q98.test Also ensure that the updated test file ends with a newline. Testing: - Manualy run modified PlannerTest that will fail and set EXTENDED_EXPLAIN test option. Verified that .test file name is printed and VERBOSE plan is not printed. - Manually run TestTpcdsDecimalV2Query with --update_results parameter and confirm the updated test file path is correct. Change-Id: I5e15af93d9016d78ac0575c433146c8513a11949 Reviewed-on: http://gerrit.cloudera.org:8080/22030 Reviewed-by: Impala Public Jenkins <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> --- .../org/apache/impala/planner/PlannerTestBase.java | 18 ++++++++++-------- .../org/apache/impala/testutil/TestFileParser.java | 8 ++++++-- tests/common/impala_test_suite.py | 17 ++++++++++++++--- tests/util/test_file_parser.py | 1 + 4 files changed, 31 insertions(+), 13 deletions(-) diff --git a/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java b/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java index 06ac6c5e0..6648adc87 100644 --- a/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java +++ b/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java @@ -454,8 +454,8 @@ public class PlannerTestBase extends FrontendTestBase { String query = testCase.getQuery(); LOG.info("running query " + query); if (query.isEmpty()) { - throw new IllegalStateException("Cannot plan empty query in line: " + - testCase.getStartingLineNum()); + throw new IllegalStateException("Cannot plan empty query in " + + testCase.getFileNameAndLineNum()); } // Set up the query context. Note that we need to deep copy it before planning each // time since planning modifies it. @@ -604,12 +604,14 @@ public class PlannerTestBase extends FrontendTestBase { String planDiff = TestUtils.compareOutput( Lists.newArrayList(explainStr.split("\n")), expectedPlan, true, resultFilters); if (!planDiff.isEmpty()) { - errorLog.append(String.format("\nSection %s of query at line %d:\n%s\n\n%s", - section, testCase.getStartingLineNum(), query, planDiff)); - // Append the VERBOSE explain plan because it contains details about - // tuples/sizes/cardinality for easier debugging. - String verbosePlan = getVerboseExplainPlan(queryCtx); - errorLog.append("\nVerbose plan:\n" + verbosePlan); + errorLog.append(String.format("\nSection %s of query at %s:\n%s\n\n%s", + section, testCase.getFileNameAndLineNum(), query, planDiff)); + if (!testOptions.contains(PlannerTestOption.EXTENDED_EXPLAIN)) { + // Append the VERBOSE explain plan because it contains details about + // tuples/sizes/cardinality for easier debugging. + String verbosePlan = getVerboseExplainPlan(queryCtx); + errorLog.append("\nVerbose plan:\n" + verbosePlan); + } } } return execRequest; diff --git a/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java b/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java index c583de8e3..457485e4a 100644 --- a/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java +++ b/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java @@ -114,16 +114,20 @@ public class TestFileParser { Maps.newEnumMap(Section.class); // Line number in the test case file where this case started + private final String fileName; private final int startLineNum; private TQueryOptions options; - public TestCase(int lineNum, TQueryOptions options) { + public TestCase(String fileName, int lineNum, TQueryOptions options) { + this.fileName = fileName; this.startLineNum = lineNum; this.options = options; } public int getStartingLineNum() { return startLineNum; } + public String getFileNameAndLineNum() { return fileName + ":" + startLineNum;} + public TQueryOptions getOptions() { return this.options; } public void setOptions(TQueryOptions options) { this.options = options; } @@ -276,7 +280,7 @@ public class TestFileParser { Section currentSection = Section.QUERY; ArrayList<String> sectionContents = Lists.newArrayList(); // Each test case in the test file has its own copy of query options. - TestCase currentTestCase = new TestCase(lineNum, options.deepCopy()); + TestCase currentTestCase = new TestCase(fileName, lineNum, options.deepCopy()); int sectionCount = 0; while (scanner.hasNextLine()) { diff --git a/tests/common/impala_test_suite.py b/tests/common/impala_test_suite.py index 05c62d278..95a5591b4 100644 --- a/tests/common/impala_test_suite.py +++ b/tests/common/impala_test_suite.py @@ -852,8 +852,14 @@ class ImpalaTestSuite(BaseTestSuite): vector.get_value('table_format').file_format, result_section='DML_RESULTS', update_section=pytest.config.option.update_results) if pytest.config.option.update_results: - output_file = os.path.join(EE_TEST_LOGS_DIR, - test_file_name.replace('/', '_') + ".test") + # Print updated test results to path like + # $EE_TEST_LOGS_DIR/impala_updated_results/tpcds/queries/tpcds-decimal_v2-q98.test + output_file = os.path.join( + EE_TEST_LOGS_DIR, 'impala_updated_results', + self.get_relative_path(self.get_workload(), test_file_name)) + output_dir = os.path.dirname(output_file) + if not os.path.exists(output_dir): + os.makedirs(output_dir) write_test_file(output_file, sections, encoding=encoding) def get_query_lineage(self, query_id, lineage_dir): @@ -1054,13 +1060,18 @@ class ImpalaTestSuite(BaseTestSuite): # Check the results assert (result is not None) and (result == expected) + def get_relative_path(self, workload, test_file_name): + """Return path to [test_file_name].test relative to WORKLOAD_DIR.""" + return os.path.join(workload, 'queries', test_file_name + '.test') + def load_query_test_file(self, workload, file_name, valid_section_names=None, encoding=None): """ Loads/Reads the specified query test file. Accepts the given section names as valid. Uses a default list of valid section names if valid_section_names is None. """ - test_file_path = os.path.join(WORKLOAD_DIR, workload, 'queries', file_name + '.test') + test_file_path = os.path.join( + WORKLOAD_DIR, self.get_relative_path(workload, file_name)) LOG.info("Loading query test file: %s", test_file_path) if not os.path.isfile(test_file_path): assert False, 'Test file not found: %s' % file_name diff --git a/tests/util/test_file_parser.py b/tests/util/test_file_parser.py index 4faf9f427..754854f49 100644 --- a/tests/util/test_file_parser.py +++ b/tests/util/test_file_parser.py @@ -364,6 +364,7 @@ def write_test_file(test_file_name, test_file_sections, encoding=None): test_file_text.append(section_value) test_file_text.append(SECTION_DELIMITER) test_file.write(('\n').join(test_file_text)) + test_file.write('\n') # end file with a newline def load_tpc_queries(workload, include_stress_queries=False, query_name_filters=[]):
