Skip to content

feat: improved assertion for easier debug#1373

Closed
sigmaaa wants to merge 1 commit into
masterfrom
terminal_test_fix
Closed

feat: improved assertion for easier debug#1373
sigmaaa wants to merge 1 commit into
masterfrom
terminal_test_fix

Conversation

@sigmaaa

@sigmaaa sigmaaa commented Dec 11, 2025

Copy link
Copy Markdown
Collaborator

Description

Please include a summary of the change and which issue is fixed.

Fixes # (IEP-XXX)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How has this been tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Test A
  • Test B

Test Configuration:

  • ESP-IDF Version:
  • OS (Windows,Linux and macOS):

Dependent components impacted by this PR:

  • Component 1
  • Component 2

Checklist

  • PR Self Reviewed
  • Applied Code formatting
  • Added Documentation
  • Added Unit Test
  • Verified on all platforms - Windows,Linux and macOS

Summary by CodeRabbit

  • Tests
    • Enhanced test error messages to provide more detailed diagnostic information for better debugging visibility.

✏️ Tip: You can customize this high-level summary in your review settings.

@sigmaaa sigmaaa self-assigned this Dec 11, 2025
@coderabbitai

coderabbitai Bot commented Dec 11, 2025

Copy link
Copy Markdown

Walkthrough

A test assertion message in TerminalViewTest.java was expanded to provide more detailed failure diagnostics. The assertion still verifies that terminal output contains "IDF v", but now displays both the expected substring and the actual output content when the assertion fails, improving test debugging visibility.

Changes

Cohort / File(s) Change Summary
Test assertion message enhancement
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/TerminalViewTest.java
Expanded assertion failure message to include expected substring and actual output content for improved diagnostic clarity

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Minor message string update with no functional logic changes

Possibly related PRs

Suggested reviewers

  • alirana01
  • kolipakakondal
  • AndriiFilippov

Poem

🐰 A message grew verbose and wise,
To show what failed before our eyes,
IDF v checked with care,
Debugging secrets laid bare! 🔍

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: improved assertion for easier debug' is directly related to the changeset, which improves a test assertion message for better debugging.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch terminal_test_fix

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/TerminalViewTest.java (1)

132-135: Improved assertion message looks good; consider removing the inline comment.

Including the expected substring and full output in the assertion message is helpful for debugging and keeps the assertion logic unchanged. The inline // Embed the actual result here comment in the middle of the concatenation is redundant and slightly hurts readability; consider deleting it or moving a brief comment above the assertion instead.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e297d94 and bc368d5.

📒 Files selected for processing (1)
  • tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/TerminalViewTest.java (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build_linux
  • GitHub Check: build_windows
  • GitHub Check: build_macos

@sigmaaa

sigmaaa commented Dec 12, 2025

Copy link
Copy Markdown
Collaborator Author

Ok, the test was actually correct and pointing to the issue with a terminal on Linux with 4.0.0, so I'm closing this

@sigmaaa sigmaaa closed this Dec 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant