WIP: SWTBot test case: Flash Process verification #1299
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughIntroduces an end-to-end SWTBot JUnit test that creates, builds, configures, and flashes a new ESP-IDF project; adds a console-based flash wait utility; and adjusts a Launch Bar selector to interact with a Canvas control for the Edit action. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant JUnit as JUnit Test
participant Fixture as Fixture
participant Env as EnvSetupOperations
participant ProjOps as ProjectTestOperations
participant LBT as LaunchBarTargetSelector
participant LBC as LaunchBarConfigSelector
participant Eclipse as Eclipse Workbench/Console
JUnit->>Fixture: loadEnv()
Fixture->>Env: setupEnvironment()
JUnit->>Fixture: createNewProject(category, subcategory, name)
Fixture->>ProjOps: createProject(...)
ProjOps-->>Fixture: project created
JUnit->>Fixture: buildProject()
Fixture->>ProjOps: triggerBuild()
ProjOps->>Eclipse: build executes
ProjOps-->>Fixture: waitForProjectBuild()
JUnit->>LBC: disable "Open Serial Monitor After Flashing"
JUnit->>LBT: selectSerialPort(target)
JUnit->>ProjOps: initiateFlash via Run Configurations
ProjOps->>Eclipse: flash executes
ProjOps-->>JUnit: waitForProjectFlash() detects "Hard resetting via RTS pin..."
JUnit->>Fixture: cleanTestEnv()
Fixture->>Eclipse: close & delete projects
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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). (1)
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
AndriiFilippov
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (6)
71-76: Replace fixed sleep with an explicit wait.Avoid brittle sleeps; use existing wait utility.
- bot.sleep(1000); + TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot);
101-106: Ensure cleanup fully quiesces jobs before and after project deletion.Add a final wait to avoid leftover background jobs affecting subsequent tests.
private static void cleanTestEnv() { TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot); ProjectTestOperations.closeAllProjects(bot); ProjectTestOperations.deleteAllProjects(bot); + TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot); }
40-51: Log full stack trace on cleanup errors.Helps diagnose teardown issues on CI.
try { Fixture.cleanTestEnv(); // Make sure test environment is always cleaned up } catch (Exception e) { - System.err.println("Error during cleanup: " + e.getMessage()); + System.err.println("Error during cleanup: " + e.getMessage()); + e.printStackTrace(); }
13-16: Remove unnecessary annotations/imports.No restricted API use and only one test method; ordering is redundant.
-import org.junit.FixMethodOrder; -import org.junit.runners.MethodSorters; @@ -@SuppressWarnings("restriction") -@FixMethodOrder(MethodSorters.NAME_ASCENDING) +Also applies to: 29-31
85-87: Make project name unique per run to avoid collisions.Stabilizes reruns on shared workspaces.
- Fixture.projectName = projectName; + Fixture.projectName = projectName + "_" + System.currentTimeMillis();
1-4: Update copyright.New file in 2025; update header.
- * Copyright 2021 Espressif Systems (Shanghai) PTE LTD. All rights reserved. + * Copyright 2021-2025 Espressif Systems (Shanghai) PTE LTD. All rights reserved.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.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). (2)
- GitHub Check: build
- GitHub Check: build_windows
...m/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java
Show resolved
Hide resolved
...m/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (1)
65-65: Fix category typo to avoid wizard lookup failure."EspressIf" → "Espressif".
- Fixture.givenNewEspressifIDFProjectIsSelected("EspressIf", "Espressif IDF Project"); + Fixture.givenNewEspressifIDFProjectIsSelected("Espressif", "Espressif IDF Project");
🧹 Nitpick comments (3)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/ProjectTestOperations.java (1)
686-694: Optional: expose a reusable consoleContains helper.Speeds up future assertions without duplicating wait logic; returns boolean instead of throwing on timeout.
+ public static boolean consoleContains(SWTWorkbenchBot bot, String consoleType, String text, long timeoutMs) throws IOException + { + SWTBotView view = viewConsole(consoleType, bot); + view.show(); + view.setFocus(); + try { + TestWidgetWaitUtility.waitUntilViewContains(bot, text, view, timeoutMs); + return true; + } catch (AssertionError ae) { + return false; + } + }Want me to wire this into the new test’s final assertion?
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (2)
141-150: Consider using direct “Flash” action to reduce UI hops.If available, invoking the project context menu “Flash” is simpler and less brittle than driving the Run Configurations dialog.
- ProjectTestOperations.launchCommandUsingContextMenu(projectName, bot, "Run Configurations..."); - TestWidgetWaitUtility.waitForDialogToAppear(bot, "Run Configurations", 10000); - bot.tree().getTreeItem("ESP-IDF Application").select(); - bot.tree().getTreeItem("ESP-IDF Application").expand(); - bot.tree().getTreeItem("ESP-IDF Application").getNode(projectName).select(); - bot.sleep(1000); - bot.button("Run").click(); + // Prefer: ProjectTestOperations.launchCommandUsingContextMenu(projectName, bot, "Flash"); + // Fallback to Run Configurations if "Flash" is not present.If “Flash” isn’t exposed in your context menu, keep the current flow.
152-154: Optional: assert explicit success markers for clearer failures.You already wait for “Hard resetting…”. Adding an assertion on a known success line (e.g., “Hash of data verified”) makes failures more diagnosable.
- ProjectTestOperations.waitProjectFlashAndVerify(bot); + ProjectTestOperations.waitProjectFlashAndVerify(bot); + // Example (if you adopt consoleContains helper): + // assertTrue("Flash success marker not found", + // ProjectTestOperations.consoleContains(bot, "ESP-IDF Console", "Hash of data verified", 30000));Want me to add the helper and wire this assertion?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java(1 hunks)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/ProjectTestOperations.java(1 hunks)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/selectors/LaunchBarTargetSelector.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/ProjectTestOperations.java (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/common/configs/DefaultPropertyFetcher.java (1)
DefaultPropertyFetcher(20-90)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (4)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/common/WorkBenchSWTBot.java (1)
WorkBenchSWTBot(14-28)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/EnvSetupOperations.java (1)
EnvSetupOperations(12-85)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/ProjectTestOperations.java (1)
ProjectTestOperations(51-774)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/selectors/LaunchBarTargetSelector.java (1)
SuppressWarnings(34-124)
...m/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java
Outdated
Show resolved
Hide resolved
...m/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java
Outdated
Show resolved
Hide resolved
...om.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/ProjectTestOperations.java
Outdated
Show resolved
Hide resolved
....idf.ui.test/src/com/espressif/idf/ui/test/operations/selectors/LaunchBarTargetSelector.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (3)
65-65: Fix category typo to avoid wizard lookup failure."EspressIf" → "Espressif".
- Fixture.givenNewEspressifIDFProjectIsSelected("EspressIf", "Espressif IDF Project"); + Fixture.givenNewEspressifIDFProjectIsSelected("Espressif", "Espressif IDF Project");
120-127: Parameterize serial port; skip test when not provided. Also wait after Finish, not before.Avoid placeholder selection and make the test deterministic in CI.
private static void whenSelectLaunchTargetSerialPort() throws Exception { LaunchBarTargetSelector targetSelector = new LaunchBarTargetSelector(bot); targetSelector.clickEdit(); TestWidgetWaitUtility.waitForDialogToAppear(bot, "New ESP Target", 20000); - bot.comboBoxWithLabel("Serial Port:").setSelection("<Once device will be connected to runner the field should be updated here>"); - TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot); - bot.button("Finish").click(); + String port = System.getProperty("idf.serial.port", System.getenv("IDF_SERIAL_PORT")); + org.junit.Assume.assumeTrue("Skipping: serial port not provided (-Didf.serial.port or IDF_SERIAL_PORT).", + port != null && !port.trim().isEmpty()); + bot.comboBoxWithLabel("Serial Port:").setSelection(port); + bot.button("Finish").click(); + TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot); }
129-137: Ensure “Open Serial Monitor After Flashing” is OFF (don’t blindly toggle).Clicking unconditionally can enable it.
private static void whenTurnOffOpenSerialMonitorAfterFlashingInLaunchConfig() throws Exception { LaunchBarConfigSelector configSelector = new LaunchBarConfigSelector(bot); configSelector.clickEdit(); TestWidgetWaitUtility.waitForDialogToAppear(bot, "Edit Configuration", 20000); bot.cTabItem("Main").show(); bot.cTabItem("Main").setFocus(); - bot.checkBox("Open Serial Monitor After Flashing").click(); + org.eclipse.swtbot.swt.finder.widgets.SWTBotCheckBox cb = bot.checkBox("Open Serial Monitor After Flashing"); + if (cb.isChecked()) { + cb.click(); + } bot.button("OK").click(); }
🧹 Nitpick comments (4)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (4)
150-152: Make success explicit with an assertion (optional).waitForProjectFlash failing will fail the test, but an assertion clarifies intent; if you add a helper like consoleContains, assert on a known success marker.
// Example (if helper exists): // assertTrue("Flash did not complete successfully", // ProjectTestOperations.consoleContains(bot, "Console", "Hard resetting via RTS pin..."));
47-58: Log full stack trace on cleanup failure.Printing only the message drops context needed to debug flaky UI tests.
catch (Exception e) { - System.err.println("Error during cleanup: " + e.getMessage()); + System.err.println("Error during cleanup:"); + e.printStackTrace(); }
113-118: Use synchronous wait before teardown actions.Ensures all jobs settle before closing/deleting projects.
private static void cleanTestEnv() { - TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot); + TestWidgetWaitUtility.waitForOperationsInProgressToFinishSync(bot); ProjectTestOperations.closeAllProjects(bot); ProjectTestOperations.deleteAllProjects(bot); }
82-88: Avoid arbitrary sleep in setup.Prefer deterministic waits; the environment setup already syncs on jobs.
EnvSetupOperations.setupEspressifEnv(bot); - bot.sleep(1000); ProjectTestOperations.deleteAllProjects(bot);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java(1 hunks)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/ProjectTestOperations.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/ProjectTestOperations.java
🧰 Additional context used
🧬 Code graph analysis (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (4)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/common/WorkBenchSWTBot.java (1)
WorkBenchSWTBot(14-28)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/EnvSetupOperations.java (1)
EnvSetupOperations(12-85)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/ProjectTestOperations.java (1)
ProjectTestOperations(51-774)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/selectors/LaunchBarTargetSelector.java (1)
SuppressWarnings(34-124)
⏰ 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). (2)
- GitHub Check: build_macos
- GitHub Check: build
🔇 Additional comments (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (1)
61-73: Scenario flow is solid.End-to-end path (create → configure → build → select port → run flash → verify) is clear and maintainable.
...m/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (2)
62-62: Fix category typo to avoid wizard lookup failure."EspressIf" → "Espressif".
- Fixture.givenNewEspressifIDFProjectIsSelected("EspressIf", "Espressif IDF Project"); + Fixture.givenNewEspressifIDFProjectIsSelected("Espressif", "Espressif IDF Project");
139-148: Clear console before launching and wait for “Run Configurations” to close. Increase enable timeout.Prevents stale-console false positives and race on dialog lifecycle.
- ProjectTestOperations.launchCommandUsingContextMenu(projectName, bot, "Run Configurations..."); - TestWidgetWaitUtility.waitForDialogToAppear(bot, "Run Configurations", 10000); - bot.tree().getTreeItem("ESP-IDF Application").select(); - bot.tree().getTreeItem("ESP-IDF Application").expand(); - bot.tree().getTreeItem("ESP-IDF Application").getNode(projectName).select(); - bot.waitUntil(widgetIsEnabled(bot.button("Run")), 5000); - bot.button("Run").click(); + ProjectTestOperations.launchCommandUsingContextMenu(projectName, bot, "Run Configurations..."); + TestWidgetWaitUtility.waitForDialogToAppear(bot, "Run Configurations", 10000); + org.eclipse.swtbot.swt.finder.widgets.SWTBotShell runCfgShell = bot.shell("Run Configurations"); + // Clear console to avoid matching old flash logs + org.eclipse.swtbot.eclipse.finder.widgets.SWTBotView consoleView = + bot.viewById("org.eclipse.ui.console.ConsoleView"); + consoleView.show(); + consoleView.toolbarButton("Clear Console").click(); + + bot.tree().getTreeItem("ESP-IDF Application").select(); + bot.tree().getTreeItem("ESP-IDF Application").expand(); + bot.tree().getTreeItem("ESP-IDF Application").getNode(projectName).select(); + bot.waitUntil(widgetIsEnabled(bot.button("Run")), 20000); + bot.button("Run").click(); + bot.waitUntil(shellCloses(runCfgShell), 20000); + TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot);
🧹 Nitpick comments (4)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (4)
126-137: Ensure dialog fully closes after changing “Open Serial Monitor After Flashing”.Great that you read state before clicking; add an explicit wait for the shell to close to reduce flakiness.
- LaunchBarConfigSelector configSelector = new LaunchBarConfigSelector(bot); - configSelector.clickEdit(); - TestWidgetWaitUtility.waitForDialogToAppear(bot, "Edit Configuration", 20000); + LaunchBarConfigSelector configSelector = new LaunchBarConfigSelector(bot); + configSelector.clickEdit(); + TestWidgetWaitUtility.waitForDialogToAppear(bot, "Edit Configuration", 20000); + final org.eclipse.swtbot.swt.finder.widgets.SWTBotShell editCfgShell = bot.shell("Edit Configuration"); bot.cTabItem("Main").show(); bot.cTabItem("Main").setFocus(); SWTBotCheckBox checkBox = bot.checkBox("Open Serial Monitor After Flashing"); if (checkBox.isChecked()) { checkBox.click(); } - bot.button("OK").click(); + bot.button("OK").click(); + bot.waitUntil(shellCloses(editCfgShell), 20000);
81-85: Avoid fixed sleep; wait for background jobs instead.Sleeping is brittle; prefer the existing async wait utility.
- bot = WorkBenchSWTBot.getBot(); - EnvSetupOperations.setupEspressifEnv(bot); - bot.sleep(1000); - ProjectTestOperations.deleteAllProjects(bot); + bot = WorkBenchSWTBot.getBot(); + EnvSetupOperations.setupEspressifEnv(bot); + TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot); + ProjectTestOperations.deleteAllProjects(bot);
35-36: Remove FixMethodOrder; it’s unnecessary with a single test and can mask ordering issues later.Keep tests independent.
-@FixMethodOrder(MethodSorters.NAME_ASCENDING) +// @FixMethodOrder removed: single test; avoid implicit ordering couplingAnd drop the unused import:
-import org.junit.FixMethodOrder; -import org.junit.runners.MethodSorters;Also applies to: 15-16
44-55: Cleanup robustness.Consider logging the full stack trace to aid CI troubleshooting.
- catch (Exception e) - { - System.err.println("Error during cleanup: " + e.getMessage()); - } + catch (Exception e) + { + e.printStackTrace(System.err); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (4)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/common/WorkBenchSWTBot.java (1)
WorkBenchSWTBot(14-28)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/EnvSetupOperations.java (1)
EnvSetupOperations(12-85)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/ProjectTestOperations.java (1)
ProjectTestOperations(51-776)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/selectors/LaunchBarTargetSelector.java (1)
SuppressWarnings(34-124)
⏰ 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). (2)
- GitHub Check: build_macos
- GitHub Check: build
🔇 Additional comments (2)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (2)
103-108: Build step orchestration looks solid.Build via context menu + explicit waits is correct and consistent with helpers.
58-70: End-to-end flow is clear and readable.Good Given/When/Then structure and helper encapsulation.
...m/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (3)
62-62: Fix wizard category typo to prevent project wizard lookup failure.
"EspressIf" → "Espressif".- Fixture.givenNewEspressifIDFProjectIsSelected("EspressIf", "Espressif IDF Project"); + Fixture.givenNewEspressifIDFProjectIsSelected("Espressif", "Espressif IDF Project");
117-125: Parameterize serial port, remove fixed sleep, and wait for dialog to close.
Make the test portable and stable; skip when no port provided.private static void whenSelectLaunchTargetSerialPort() throws Exception { LaunchBarTargetSelector targetSelector = new LaunchBarTargetSelector(bot); targetSelector.clickEdit(); TestWidgetWaitUtility.waitForDialogToAppear(bot, "New ESP Target", 20000); - bot.comboBoxWithLabel("Serial Port:").setSelection("/dev/ttyUSB1 CP2102N USB to UART Bridge Controller"); -// TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot); - bot.sleep(5000); - bot.button("Finish").click(); + final org.eclipse.swtbot.swt.finder.widgets.SWTBotShell targetShell = bot.shell("New ESP Target"); + final String port = System.getProperty("idf.serial.port", System.getenv("IDF_SERIAL_PORT")); + org.junit.Assume.assumeTrue( + "Skipping: serial port not provided (-Didf.serial.port or IDF_SERIAL_PORT).", + port != null && !port.trim().isEmpty()); + bot.comboBoxWithLabel("Serial Port:").setSelection(port.trim()); + TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot); + bot.button("Finish").click(); + bot.waitUntil(shellCloses(targetShell), 20000); }
140-149: Clear console before launching and wait for “Run Configurations” to close.
Prevents stale matches and flakiness.private static void whenFlashProject() throws IOException { ProjectTestOperations.launchCommandUsingContextMenu(projectName, bot, "Run Configurations..."); TestWidgetWaitUtility.waitForDialogToAppear(bot, "Run Configurations", 10000); bot.tree().getTreeItem("ESP-IDF Application").select(); bot.tree().getTreeItem("ESP-IDF Application").expand(); bot.tree().getTreeItem("ESP-IDF Application").getNode(projectName).select(); - bot.waitUntil(widgetIsEnabled(bot.button("Run")), 5000); - bot.button("Run").click(); + org.eclipse.swtbot.eclipse.finder.widgets.SWTBotView consoleView = bot.viewById("org.eclipse.ui.console.ConsoleView"); + consoleView.show(); + consoleView.toolbarButton("Clear Console").click(); + org.eclipse.swtbot.swt.finder.widgets.SWTBotShell runCfgShell = bot.shell("Run Configurations"); + bot.waitUntil(widgetIsEnabled(bot.button("Run")), 20000); + bot.button("Run").click(); + bot.waitUntil(shellCloses(runCfgShell), 20000); }
🧹 Nitpick comments (4)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (4)
127-138: Also wait for “Edit Configuration” dialog to close after clicking OK.
Improves determinism before proceeding.private static void whenTurnOffOpenSerialMonitorAfterFlashingInLaunchConfig() throws Exception { LaunchBarConfigSelector configSelector = new LaunchBarConfigSelector(bot); configSelector.clickEdit(); TestWidgetWaitUtility.waitForDialogToAppear(bot, "Edit Configuration", 20000); + final org.eclipse.swtbot.swt.finder.widgets.SWTBotShell editCfgShell = bot.shell("Edit Configuration"); bot.cTabItem("Main").show(); bot.cTabItem("Main").setFocus(); SWTBotCheckBox checkBox = bot.checkBox("Open Serial Monitor After Flashing"); if (checkBox.isChecked()) { checkBox.click(); } bot.button("OK").click(); + bot.waitUntil(shellCloses(editCfgShell), 20000); }
44-55: Don’t swallow cleanup exceptions; fail the test to surface leaks.
Hidden cleanup errors can poison subsequent runs.@After public void afterEachTest() { try { Fixture.cleanTestEnv(); // Make sure test environment is always cleaned up } catch (Exception e) { - System.err.println("Error during cleanup: " + e.getMessage()); + e.printStackTrace(System.err); + org.junit.Assert.fail("Cleanup failed: " + e.getMessage()); } }
81-85: Avoid fixed sleep in setup; wait for jobs instead.
Reduces flakiness and speeds up.EnvSetupOperations.setupEspressifEnv(bot); - bot.sleep(1000); - ProjectTestOperations.deleteAllProjects(bot); + TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot); + ProjectTestOperations.deleteAllProjects(bot);
33-36: Drop @FixMethodOrder unless you rely on order.
With a single test, this is unnecessary noise.-@FixMethodOrder(MethodSorters.NAME_ASCENDING)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (4)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/common/WorkBenchSWTBot.java (1)
WorkBenchSWTBot(14-28)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/EnvSetupOperations.java (1)
EnvSetupOperations(12-85)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/ProjectTestOperations.java (1)
ProjectTestOperations(51-776)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/selectors/LaunchBarTargetSelector.java (1)
SuppressWarnings(34-124)
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: build_macos
🔇 Additional comments (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (1)
131-136: Good: explicitly ensure “Open Serial Monitor After Flashing” is OFF.
This avoids accidentally enabling the monitor via blind toggles.
AndriiFilippov
force-pushed
the
IEP-1626
branch
from
0cf1f7c to
ceea67b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (3)
62-62: Fix “EspressIf” typo to match wizard categoryThis prevents the wizard selection from failing.
- Fixture.givenNewEspressifIDFProjectIsSelected("EspressIf", "Espressif IDF Project"); + Fixture.givenNewEspressifIDFProjectIsSelected("Espressif", "Espressif IDF Project");
117-124: Parameterize serial port and wait for dialog to closeHard-coded port breaks CI portability; also ensure the “New ESP Target” shell closes before proceeding.
private static void whenSelectLaunchTargetSerialPort() throws Exception { LaunchBarTargetSelector targetSelector = new LaunchBarTargetSelector(bot); targetSelector.clickEdit(); TestWidgetWaitUtility.waitForDialogToAppear(bot, "New ESP Target", 20000); - bot.comboBoxWithLabel("Serial Port:").setSelection("/dev/ttyUSB1 CP2102N USB to UART Bridge Controller"); - TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot); - bot.button("Finish").click(); + final org.eclipse.swtbot.swt.finder.widgets.SWTBotShell targetShell = bot.shell("New ESP Target"); + final String port = System.getProperty("idf.serial.port", + System.getenv("IDF_SERIAL_PORT")); + org.junit.Assume.assumeTrue( + "Skipping: serial port not provided (-Didf.serial.port or IDF_SERIAL_PORT).", + port != null && !port.trim().isEmpty()); + bot.comboBoxWithLabel("Serial Port:").setSelection(port.trim()); + TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot); + bot.button("Finish").click(); + bot.waitUntil(shellCloses(targetShell), 20000); }
139-148: Clear console and wait for “Run Configurations” to close; increase waitPrevents stale-console false positives and reduces flakiness.
private static void whenFlashProject() throws IOException { ProjectTestOperations.launchCommandUsingContextMenu(projectName, bot, "Run Configurations..."); TestWidgetWaitUtility.waitForDialogToAppear(bot, "Run Configurations", 10000); + final org.eclipse.swtbot.swt.finder.widgets.SWTBotShell runCfgShell = bot.shell("Run Configurations"); + // Start with a clean console + org.eclipse.swtbot.eclipse.finder.widgets.SWTBotView consoleView = bot.viewById("org.eclipse.ui.console.ConsoleView"); + consoleView.show(); + consoleView.toolbarButton("Clear Console").click(); bot.tree().getTreeItem("ESP-IDF Application").select(); bot.tree().getTreeItem("ESP-IDF Application").expand(); bot.tree().getTreeItem("ESP-IDF Application").getNode(projectName).select(); - bot.waitUntil(widgetIsEnabled(bot.button("Run")), 5000); - bot.button("Run").click(); + bot.waitUntil(widgetIsEnabled(bot.button("Run")), 20000); + bot.button("Run").click(); + bot.waitUntil(shellCloses(runCfgShell), 20000); }
🧹 Nitpick comments (2)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (2)
79-85: Avoid fixed sleep; wait for background jobs insteadImproves stability and removes arbitrary delay.
EnvSetupOperations.setupEspressifEnv(bot); - bot.sleep(1000); + TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot); ProjectTestOperations.deleteAllProjects(bot);
15-18: Remove FixMethodOrder; unnecessary for a single testSimplifies annotations; order control isn’t needed here.
-import org.junit.FixMethodOrder; @@ -import org.junit.runners.MethodSorters; @@ -@FixMethodOrder(MethodSorters.NAME_ASCENDING)Also applies to: 35-35
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (4)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/common/WorkBenchSWTBot.java (1)
WorkBenchSWTBot(14-28)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/EnvSetupOperations.java (1)
EnvSetupOperations(12-85)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/ProjectTestOperations.java (1)
ProjectTestOperations(51-776)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/selectors/LaunchBarTargetSelector.java (1)
SuppressWarnings(34-124)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (2)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (2)
126-137: Good: explicitly ensure Serial Monitor is OFFYou read the checkbox state and only uncheck if needed, preventing accidental toggles.
150-152: LGTM: wait for flash success markerDelegating to ProjectTestOperations.waitForProjectFlash(bot) is appropriate.
...m/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java
Outdated
Show resolved
Hide resolved
AndriiFilippov
force-pushed
the
IEP-1626
branch
2 times, most recently
from
09ff318 to
ceea67b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (2)
62-62: Fix category typo to avoid wizard lookup failure."EspressIf" → "Espressif".
- Fixture.givenNewEspressifIDFProjectIsSelected("EspressIf", "Espressif IDF Project"); + Fixture.givenNewEspressifIDFProjectIsSelected("Espressif", "Espressif IDF Project");
117-124: Parameterize serial port and skip when unset; also wait for dialog to close.Avoid hard-coding device names; makes tests portable and CI-friendly.
- LaunchBarTargetSelector targetSelector = new LaunchBarTargetSelector(bot); - targetSelector.clickEdit(); - TestWidgetWaitUtility.waitForDialogToAppear(bot, "New ESP Target", 20000); - bot.comboBoxWithLabel("Serial Port:").setSelection("/dev/ttyUSB1 Dual RS232-HS"); - TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot); - bot.button("Finish").click(); + LaunchBarTargetSelector targetSelector = new LaunchBarTargetSelector(bot); + targetSelector.clickEdit(); + TestWidgetWaitUtility.waitForDialogToAppear(bot, "New ESP Target", 20000); + final org.eclipse.swtbot.swt.finder.widgets.SWTBotShell targetShell = bot.shell("New ESP Target"); + final String port = System.getProperty("idf.serial.port", System.getenv("IDF_SERIAL_PORT")); + org.junit.Assume.assumeTrue("Skipping: serial port not provided (-Didf.serial.port or IDF_SERIAL_PORT).", + port != null && !port.trim().isEmpty()); + bot.comboBoxWithLabel("Serial Port:").setSelection(port.trim()); + TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot); + bot.button("Finish").click(); + bot.waitUntil(shellCloses(targetShell), 20000);
🧹 Nitpick comments (3)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (3)
139-148: Clear console before running and wait for Run Configurations to close.Prevents stale-output false positives and reduces race conditions.
- ProjectTestOperations.launchCommandUsingContextMenu(projectName, bot, "Run Configurations..."); - TestWidgetWaitUtility.waitForDialogToAppear(bot, "Run Configurations", 10000); - bot.tree().getTreeItem("ESP-IDF Application").select(); - bot.tree().getTreeItem("ESP-IDF Application").expand(); - bot.tree().getTreeItem("ESP-IDF Application").getNode(projectName).select(); - bot.waitUntil(widgetIsEnabled(bot.button("Run")), 5000); - bot.button("Run").click(); + ProjectTestOperations.launchCommandUsingContextMenu(projectName, bot, "Run Configurations..."); + TestWidgetWaitUtility.waitForDialogToAppear(bot, "Run Configurations", 10000); + bot.tree().getTreeItem("ESP-IDF Application").select(); + bot.tree().getTreeItem("ESP-IDF Application").expand(); + bot.tree().getTreeItem("ESP-IDF Application").getNode(projectName).select(); + // Clear console to avoid stale matches + org.eclipse.swtbot.eclipse.finder.widgets.SWTBotView consoleView = bot.viewById("org.eclipse.ui.console.ConsoleView"); + consoleView.show(); + consoleView.toolbarButton("Clear Console").click(); + // Click Run and wait for dialog to close + org.eclipse.swtbot.swt.finder.widgets.SWTBotShell runCfgShell = bot.shell("Run Configurations"); + bot.waitUntil(widgetIsEnabled(bot.button("Run")), 20000); + bot.button("Run").click(); + bot.waitUntil(shellCloses(runCfgShell), 20000);
44-55: Optional: move cleanup to @afterclass if more tests are added.Reduces redundant teardown when multiple tests exist in this class.
- @After - public void afterEachTest() + @org.junit.AfterClass + public static void afterTestClass()
79-85: Replace fixed sleep with a deterministic wait.Avoid arbitrary delays; rely on existing wait utilities.
- bot.sleep(1000); + TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (4)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/common/WorkBenchSWTBot.java (1)
WorkBenchSWTBot(14-28)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/EnvSetupOperations.java (1)
EnvSetupOperations(12-85)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/ProjectTestOperations.java (1)
ProjectTestOperations(51-776)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/selectors/LaunchBarTargetSelector.java (1)
SuppressWarnings(34-124)
⏰ 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). (1)
- GitHub Check: build_macos
🔇 Additional comments (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (1)
126-137: Good: explicitly ensure “Open Serial Monitor After Flashing” is OFF.Avoids accidental toggling; matches prior feedback.
AndriiFilippov
force-pushed
the
IEP-1626
branch
2 times, most recently
from
48bf344 to
fa049a6
Compare
Collaborator
Author
|
@alirana01 please, review |
5 tasks
5 tasks
5 tasks
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Flash process verification tests
Fixes # (IEP-1626)
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:
Checklist
Summary by CodeRabbit