TPCH: Try to improve logging for invalid values

[Swiddis]

Description

#5103 is still failing, if it's actually a value mismatch then it's not being shown due to bad assert logging. This fixes the logging.

Related Issues

#5103, #4261

Check List

• New functionality includes testing.
• New functionality has been documented.
• New functionality has javadoc added.
• New functionality has a user manual doc added.
• New PPL command checklist all confirmed.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff or -s.
• Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Tests
  • Improved test failure messages to display actual expected values instead of placeholders, enabling more informative diagnostics during testing.

Walkthrough

A single-line change to a test utility's matcher that improves the error message display. The modification updates how expected values are represented in test failure messages, changing from an empty list placeholder to the actual values array.

Changes

Cohort / File(s)	Summary
Test Utility Improvement integ-test/src/test/java/org/opensearch/sql/util/MatcherUtils.java	Modified the describeTo method in closeTo matcher to display actual expected values array instead of a placeholder, improving test failure message clarity.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Suggested labels

flaky-test, skip-changelog

Suggested reviewers

• kavithacm
• derek-ho
• penghuo
• seankao-az
• anirudha

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is related to the changeset but is overly broad and doesn't accurately capture the specific change, which is fixing the matcher logging representation rather than general TPCH logging.
Description check	✅ Passed	The description is related to the changeset as it references the logging issue that the code change addresses, explaining the context and purpose of the fix.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@integ-test/src/test/java/org/opensearch/sql/util/MatcherUtils.java`:
- Around line 330-331: Add a unit test that exercises the closeTo matcher and
asserts its failure description contains the expected values array string
produced by MatcherUtils.describeTo; specifically, create a test that constructs
the closeTo matcher (use the same factory or method used in tests), call
describeTo on a Description (e.g., StringDescription), and assert the resulting
description string includes the array text "[1.0, 2.0]"; reference the
describeTo method in class MatcherUtils and the closeTo matcher so the test
verifies the new Arrays.toString(values) output and prevents regressions.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the full context of the MatcherUtils.closeTo implementation
cat -n integ-test/src/test/java/org/opensearch/sql/util/MatcherUtils.java | sed -n '310,350p'

Repository: opensearch-project/sql

Length of output: 1860

---

🏁 Script executed:

# Search for test files that test MatcherUtils
fd -t f -e java -path '*/test*' | xargs grep -l 'MatcherUtils\|closeTo' 2>/dev/null | head -20

Repository: opensearch-project/sql

Length of output: 234

---

🏁 Script executed:

# Search for tests that specifically test describeTo or assertion messages for closeTo
rg -n 'closeTo|describeTo' --glob '*Test*.java' --glob '*IT*.java' -B 2 -A 5

Repository: opensearch-project/sql

Length of output: 50378

---

🏁 Script executed:

# Search for test files that specifically test MatcherUtils
find . -name "*MatcherUtils*Test*.java" -o -name "*Test*MatcherUtils*.java" 2>/dev/null

Repository: opensearch-project/sql

Length of output: 48

---

🏁 Script executed:

# Search for any test that validates describeTo output or failure messages for MatcherUtils
rg -n 'MatcherUtils\s*\(' integ-test/src/test/ -A 5 | grep -E 'describeTo|failure|message|assert.*describe' -i | head -30

Repository: opensearch-project/sql

Length of output: 48

---

🏁 Script executed:

# Look for test files in integ-test/src/test that might test MatcherUtils directly
fd -type f -name "*.java" integ-test/src/test | xargs grep -l "class.*MatcherUtils.*Test" 2>/dev/null

Repository: opensearch-project/sql

Length of output: 238

---

🏁 Script executed:

# Search for any test files that import or reference MatcherUtils to see where it's tested
rg -l "import.*MatcherUtils|from.*MatcherUtils" --glob "*.java" integ-test/src/test | head -10

Repository: opensearch-project/sql

Length of output: 803

---

🏁 Script executed:

# Check if there are tests that assert matcher descriptions/failure messages
rg -n "describeTo|Matcher.*describe|failure.*message" --glob "*Test*.java" integ-test/src/test -i | head -20

Repository: opensearch-project/sql

Length of output: 309

---

🏁 Script executed:

# Look at the MatcherUtils file to see all matcher implementations and check for existing test patterns
wc -l integ-test/src/test/java/org/opensearch/sql/util/MatcherUtils.java

Repository: opensearch-project/sql

Length of output: 136

---

🏁 Script executed:

# Check if the entire MatcherUtils file has any embedded tests or test helpers
grep -n "public static.*Matcher\|class.*Test" integ-test/src/test/java/org/opensearch/sql/util/MatcherUtils.java | head -20

Repository: opensearch-project/sql

Length of output: 1843

---

🏁 Script executed:

# Search for tests that specifically test closeTo matcher's output/description
rg -n "closeTo.*describe|closeTo.*fail|closeTo.*message" --glob "*Test*.java" --glob "*IT*.java" integ-test/src/test

Repository: opensearch-project/sql

Length of output: 48

---

🏁 Script executed:

# Check if there are assertion tests that validate the matcher failure message format
rg -B 3 -A 3 "assertThat.*matcher|assertThat.*describe" --glob "*Test*.java" integ-test/src/test -i | head -40

Repository: opensearch-project/sql

Length of output: 48

---

🏁 Script executed:

# Look for pattern: tests that might validate what the matcher says when it fails
rg -n "mismatch|fail.*Matcher|Matcher.*fail" --glob "*Test*.java" integ-test/src/test -i | head -20

Repository: opensearch-project/sql

Length of output: 646

---

🏁 Script executed:

# Check if MatcherUtils.closeTo is mentioned in any test assertions or test method names
rg -n "testCloseTo|closeTo.*Test|Test.*closeTo" --glob "*.java" integ-test/src/test -i

Repository: opensearch-project/sql

Length of output: 48

---

🏁 Script executed:

# Let's verify the complete context of closeTo implementation and surrounding code
cat -n integ-test/src/test/java/org/opensearch/sql/util/MatcherUtils.java | sed -n '300,350p'

Repository: opensearch-project/sql

Length of output: 2174

---

🏁 Script executed:

# Check if there are any test classes in the core or other modules that test MatcherUtils
find . -path "*/test/*" -name "*MatcherUtils*.java" -o -path "*/test/*" -name "*Test*.java" | xargs grep -l "MatcherUtils.closeTo" 2>/dev/null | head -5

Repository: opensearch-project/sql

Length of output: 48

---

🏁 Script executed:

# Search for any unit tests that use MatcherAssert with closeTo
rg -l "assertThat.*MatcherUtils.closeTo" --glob "*Test.java" --glob "*IT.java" | head -5

Repository: opensearch-project/sql

Length of output: 48

---

Add unit tests for the closeTo matcher's failure message format.

The describeTo method now outputs Arrays.toString(values) to display expected values. Add a unit test that asserts the matcher's description includes the expected values array (e.g., [1.0, 2.0]) to validate the failure message format and prevent regressions.

🤖 Prompt for AI Agents

In `@integ-test/src/test/java/org/opensearch/sql/util/MatcherUtils.java` around
lines 330 - 331, Add a unit test that exercises the closeTo matcher and asserts
its failure description contains the expected values array string produced by
MatcherUtils.describeTo; specifically, create a test that constructs the closeTo
matcher (use the same factory or method used in tests), call describeTo on a
Description (e.g., StringDescription), and assert the resulting description
string includes the array text "[1.0, 2.0]"; reference the describeTo method in
class MatcherUtils and the closeTo matcher so the test verifies the new
Arrays.toString(values) output and prevents regressions.