DRAFT: IEP-1559 Add SWT Tests to Verify Syntax Highlighting Functionality #1255
Conversation
WalkthroughA new UI test was added to the Espressif IDF project test suite. This test creates a project, opens the main source file, and checks the Eclipse Error Log for exceptions. Supporting logic was implemented in a new static method within an inner class to encapsulate the UI interactions required for the test. Changes
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectTest.java(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: precommit
- GitHub Check: build_macos
| @Test | ||
| public void givenMainCFileOpenedThenNoErrorsShouldBeInErrorLog() throws Exception | ||
| { | ||
| Fixture.givenNewEspressifIDFProjectIsSelected("EspressIf", "Espressif IDF Project"); | ||
| Fixture.givenProjectNameIs("ErrorLogTestProject"); | ||
| Fixture.whenNewProjectIsSelected(); | ||
| Fixture.openMainCAndCheckIfNoExceptionInErrorLog(); | ||
| } |
There was a problem hiding this comment.
Potential flakiness – Error Log may contain residual entries from previous tests
givenMainCFileOpenedThenNoErrorsShouldBeInErrorLog assumes that the Eclipse Error Log starts empty, yet none of the test-fixture methods clear the log first.
If an earlier test has left warnings or exceptions in the log (even if unrelated), this test will fail non-deterministically, making the suite flaky.
Add a step to clear the Error Log before performing the assertion, or clear it in Fixture.cleanTestEnv().
Example (inside the new helper method before the assertion):
errorLogView.toolbarButton("Clear Log").click();🤖 Prompt for AI Agents
In
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectTest.java
around lines 184 to 191, the test
givenMainCFileOpenedThenNoErrorsShouldBeInErrorLog assumes the Eclipse Error Log
is empty but does not clear it before checking, causing potential flakiness. Fix
this by adding a step to clear the Error Log before the assertion, either by
invoking errorLogView.toolbarButton("Clear Log").click() before opening the main
C file or by clearing the log in Fixture.cleanTestEnv() to ensure a clean state
for the test.
| try | ||
| { | ||
| SWTBotTable errorTable = errorLogView.bot().table(); | ||
| // Check if there are no exceptions in error log | ||
| boolean hasErrors = errorTable.containsItem("Exception"); | ||
|
|
||
| assertTrue(!hasErrors); | ||
| } | ||
| catch (WidgetNotFoundException e) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Assertion can miss errors and lacks clarity
SWTBotTable.containsItem("Exception") only matches if any cell equals “Exception”.
Actual log rows usually contain class names or messages ("java.lang.Exception", "Unhandled event loop exception", …) and will not be caught.
Replace the check with a scan of all rows for the substring “Exception” (or simply assert that the table has zero rows), and use assertFalse for readability:
- boolean hasErrors = errorTable.containsItem("Exception");
- assertTrue(!hasErrors);
+ boolean hasErrors = Arrays.stream(errorTable.getItems())
+ .anyMatch(item -> item.getText().contains("Exception"));
+ assertFalse("Unexpected entries containing 'Exception' found in the Error Log", hasErrors);You will need to add
import static org.junit.Assert.assertFalse;to the import section.
🤖 Prompt for AI Agents
In
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectTest.java
around lines 437 to 445, the current assertion uses
SWTBotTable.containsItem("Exception") which only matches exact cell content and
can miss error messages containing "Exception" as a substring. Replace this with
code that scans all rows for the substring "Exception" or simply assert that the
error table has zero rows to ensure no errors are present. Also, change the
assertion to assertFalse for better readability and add the import statement
'import static org.junit.Assert.assertFalse;' to the imports section.
| // Open the "Error Log" view | ||
| bot.menu("Window").menu("Show View").menu("Other...").click(); | ||
|
|
||
| // Type and open "Error Log" | ||
| SWTBotShell showViewShell = bot.shell("Show View"); | ||
| showViewShell.activate(); | ||
| bot.tree().expandNode("General").select("Error Log"); | ||
| bot.button("Open").click(); | ||
| SWTBotView errorLogView = bot.activeView(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Menu navigation is brittle across Eclipse versions
Opening Window ▸ Show View ▸ Other… and then drilling down to “General → Error Log” relies on the view being nested exactly there.
On recent Eclipse releases the Error Log is often available directly under Show View (without Other…) or lives in a different category.
Recommend falling back to direct access first and only opening the Other… dialog if the direct menu item is absent:
- bot.menu("Window").menu("Show View").menu("Other...").click();
+ SWTBotMenu showView = bot.menu("Window").menu("Show View");
+ if (showView.menu("Error Log", false) != null) { // false → do not fail if missing
+ showView.menu("Error Log").click();
+ } else {
+ showView.menu("Other...").click();
+ // … existing code to open via dialog
+ }This removes a common source of WidgetNotFoundException on different IDE versions.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Open the "Error Log" view | |
| bot.menu("Window").menu("Show View").menu("Other...").click(); | |
| // Type and open "Error Log" | |
| SWTBotShell showViewShell = bot.shell("Show View"); | |
| showViewShell.activate(); | |
| bot.tree().expandNode("General").select("Error Log"); | |
| bot.button("Open").click(); | |
| SWTBotView errorLogView = bot.activeView(); | |
| // Open the "Error Log" view | |
| SWTBotMenu showView = bot.menu("Window").menu("Show View"); | |
| if (showView.menu("Error Log", false) != null) { | |
| showView.menu("Error Log").click(); | |
| } else { | |
| showView.menu("Other...").click(); | |
| // … existing code to open via dialog | |
| } | |
| // Type and open "Error Log" | |
| SWTBotShell showViewShell = bot.shell("Show View"); | |
| showViewShell.activate(); | |
| bot.tree().expandNode("General").select("Error Log"); | |
| bot.button("Open").click(); | |
| SWTBotView errorLogView = bot.activeView(); |
🤖 Prompt for AI Agents
In
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectTest.java
around lines 427 to 435, the current code opens the "Error Log" view by
navigating through "Window ▸ Show View ▸ Other…" and then selecting "General →
Error Log," which is brittle across Eclipse versions. To fix this, first attempt
to open the "Error Log" view directly from the "Show View" menu if available; if
not found, then fall back to opening the "Other…" dialog and selecting "Error
Log" from there. This approach avoids WidgetNotFoundException by adapting to
different menu structures in various Eclipse versions.
There was a problem hiding this comment.
LGTM!
Its good that we are not using any example projects since they may have some dependency that is missing on test env. Do you think we can improve it to have some predefined main.c file that can reference some internal files specifically I think FreeRTOS area because they are the ones that can give most errors
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.
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 Configuration:
Dependent components impacted by this PR:
Checklist
Summary by CodeRabbit