tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/IDFProjectDebugProcessTest.java (4)

66-84: Platform-specific test execution needs documentation.

The Linux-only execution with a fallback assertTrue(true) for non-Linux platforms is a reasonable temporary approach, but it should be documented more clearly in the code and potentially in the test method name.

Consider adding a more informative comment and potentially using JUnit's Assume for cleaner platform filtering:

+import org.junit.Assume;

 @Test
 public void givenNewProjectCreatedWhenFlashedAndDebuggedThenDebuggingWorks() throws Exception
 {
-	if (SystemUtils.IS_OS_LINUX) //temporary solution until new ESP boards arrive for Windows
+	// Skip test on non-Linux platforms until ESP boards are available for Windows/macOS
+	Assume.assumeTrue("Test requires Linux platform with ESP hardware", SystemUtils.IS_OS_LINUX);
+	
 	{
 		Fixture.givenNewEspressifIDFProjectIsSelected("EspressIf", "Espressif IDF Project");
 		// ... rest of test logic
-	}
-	else
-	{
-		assertTrue(true);
-	}
+	}
 }

---

168-170: Inconsistent code formatting.

The indentation inside the if statement is inconsistent with the rest of the codebase.

Apply this diff to fix the formatting:

 if (checkBox.isChecked()) {
-checkBox.click();
-}
+	checkBox.click();
+}

---

148-148: Hardcoded board selection may cause test brittleness.

The hardcoded board selection "ESP32-ETHERNET-KIT [usb://1-10]" assumes a specific USB port and board availability, which could make the test brittle in different environments.

Consider making the board selection more flexible or add error handling for cases where the expected board is not available:

-bot.comboBoxWithLabel("Board:").setSelection("ESP32-ETHERNET-KIT [usb://1-10]");
+// Try to select ESP32-ETHERNET-KIT, fallback to first available board if not found
+SWTBotComboBox boardCombo = bot.comboBoxWithLabel("Board:");
+String[] items = boardCombo.items();
+String targetBoard = "ESP32-ETHERNET-KIT [usb://1-10]";
+boolean boardFound = false;
+for (String item : items) {
+    if (item.contains("ESP32-ETHERNET-KIT")) {
+        boardCombo.setSelection(item);
+        boardFound = true;
+        break;
+    }
+}
+if (!boardFound && items.length > 0) {
+    boardCombo.setSelection(0); // Select first available board
+}

---

180-180: Hardcoded serial port selection needs flexibility.

Similar to the board selection, the hardcoded serial port "/dev/ttyUSB1 Dual RS232-HS" assumes a specific hardware configuration that may not be available in all test environments.

Consider adding logic to select the first available serial port if the hardcoded one is not found:

-bot.comboBoxWithLabel("Serial Port:").setSelection("/dev/ttyUSB1 Dual RS232-HS");
+// Try to select preferred serial port, fallback to first available if not found  
+SWTBotComboBox portCombo = bot.comboBoxWithLabel("Serial Port:");
+String[] items = portCombo.items();
+String preferredPort = "/dev/ttyUSB1 Dual RS232-HS";
+boolean portFound = false;
+for (String item : items) {
+    if (item.equals(preferredPort)) {
+        portCombo.setSelection(item);
+        portFound = true;
+        break;
+    }
+}
+if (!portFound && items.length > 0) {
+    portCombo.setSelection(0); // Select first available port
+}

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/IDFProjectDebugProcessTest.java (2)

82-86: Core debug flow is commented out — implement or explicitly scope the test
The test currently stops after selecting the debug config/target. Either enable the debug launch and add assertions (e.g., debugger attaches, expected nodes appear in Debug view), or update the PR to clarify this test only covers configuration/flash. Otherwise it doesn’t meet the stated objective.

---

73-73: Project name typo

Repeated from a prior review: fix “NewProjecDebugTest” → “NewProjectDebugTest”.

-            Fixture.givenProjectNameIs("NewProjecDebugTest");
+            Fixture.givenProjectNameIs("NewProjectDebugTest");

tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/IDFProjectDebugProcessTest.java (5)

8-10: Use JUnit Assume to skip on non‑Linux instead of passing trivially

Add an import for Assume.

 import static org.eclipse.swtbot.swt.finder.waits.Conditions.widgetIsEnabled;
+import static org.junit.Assume.assumeTrue;
 import static org.junit.Assert.assertTrue;

---

67-91: Replace if/else OS gate with an assumption

Avoid a vacuous pass on non‑Linux; skip the test instead.

-        if (SystemUtils.IS_OS_LINUX) // temporary solution until new ESP boards arrive for Windows
-        {
+        assumeTrue("Linux-only test path until Windows boards are available", SystemUtils.IS_OS_LINUX);
             Fixture.givenNewEspressifIDFProjectIsSelected("EspressIf", "Espressif IDF Project");
             Fixture.givenProjectNameIs("NewProjecDebugTest");
             Fixture.whenNewProjectIsSelected();
             Fixture.whenTurnOffOpenSerialMonitorAfterFlashingInLaunchConfig();
             Fixture.whenSelectLaunchTargetSerialPort();
             Fixture.whenProjectIsBuiltUsingContextMenu();
             Fixture.whenFlashProject();
             Fixture.thenVerifyFlashDoneSuccessfully();
             Fixture.whenSelectDebugConfig();
             Fixture.whenSelectLaunchTargetBoard();
 //          Fixture.whenDebugProject();
 //          Fixture.whenSwitchPerspective();
 //          Fixture.checkIfOpenOCDandGDBprocessesArePresent();
 //          Fixture.whenDebugStoppedUsingContextMenu();
-
-        }
-        else
-        {
-            assertTrue(true);
-        }

Note: Keep the typo fixes from the other comments when applying this change.

---

148-158: Hard‑coded board label — parameterize via properties with fallback

Using a fixed “ESP32-ETHERNET-KIT [usb://1-10]” is brittle across labs. Read from DefaultPropertyFetcher with a safe default.

-            bot.comboBoxWithLabel("Board:").setSelection("ESP32-ETHERNET-KIT [usb://1-10]");
+            String desiredBoard = DefaultPropertyFetcher.getStringPropertyValue("test.launch.board.label", "");
+            if (desiredBoard != null && !desiredBoard.isBlank()) {
+                bot.comboBoxWithLabel("Board:").setSelection(desiredBoard);
+            } else {
+                // fallback to first available item
+                String[] items = bot.comboBoxWithLabel("Board:").items();
+                bot.comboBoxWithLabel("Board:").setSelection(items.length > 0 ? items[0] : "");
+            }

Consider documenting the new property key (e.g., in test README). Based on learnings.

---

182-192: Hard‑coded serial port — parameterize via properties with fallback

Make the serial port configurable to avoid machine‑specific failures.

-            bot.comboBoxWithLabel("Serial Port:").setSelection("/dev/ttyUSB1 Dual RS232-HS");
+            String desiredPort = DefaultPropertyFetcher.getStringPropertyValue("test.launch.serial.port.label", "");
+            if (desiredPort != null && !desiredPort.isBlank()) {
+                bot.comboBoxWithLabel("Serial Port:").setSelection(desiredPort);
+            } else {
+                String[] ports = bot.comboBoxWithLabel("Serial Port:").items();
+                bot.comboBoxWithLabel("Serial Port:").setSelection(ports.length > 0 ? ports[0] : "");
+            }

---

54-65: Surface cleanup failures for diagnostics

Print the stack trace to aid triage instead of only the message.

-            System.err.println("Error during cleanup: " + e.getMessage());
+            System.err.println("Error during cleanup: " + e.getMessage());
+            e.printStackTrace(System.err);

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/IDFProjectDebugProcessTest.java (3)

72-72: Fix critical typos in project setup.

Two typos remain from previous reviews that will break the test:

1. Category label "EspressIf" should be "Espressif" to match the wizard UI
2. Project name "NewProjecDebugTest" should be "NewProjectDebugTest"

Apply this diff to fix both typos:

-			Fixture.createNewEspressifProject("EspressIf", "Espressif IDF Project", "NewProjecDebugTest");
+			Fixture.createNewEspressifProject("Espressif", "Espressif IDF Project", "NewProjectDebugTest");

---

258-258: Fix critical string match bug (missing space).

The filter checks for projectName + "Debug [ESP-IDF GDB OpenOCD Debugging]" but the actual debug configuration name is projectName + " Debug [ESP-IDF GDB OpenOCD Debugging]" (with a space before "Debug"). This will never match and the test will fail.

Apply this diff to fix the match:

-					.filter(i -> i.getText().equals(projectName + "Debug [ESP-IDF GDB OpenOCD Debugging]"))
+					.filter(i -> i.getText().equals(projectName + " Debug [ESP-IDF GDB OpenOCD Debugging]"))

---

110-119: Test does not verify debug configuration fields per PR objectives.

The PR description states the test should verify "the match of the project name, Actual Executable, and SVD Path in the New ESP-IDF GDB OpenOCD Debugging Launch Configuration". The current implementation only selects the debug configuration and launches it, but does not assert these fields.

Consider adding verification steps in whenDebugProject() or a new thenVerifyDebugConfiguration() method that:

1. Opens the Debug Configurations dialog
2. Selects the ESP-IDF GDB OpenOCD Debugging configuration for the project
3. Asserts the "Project" field matches the expected project name
4. Asserts the "Actual Executable" path is valid
5. Asserts the "SVD Path" is set correctly

You can retrieve these values from the dialog's text fields before clicking "Debug". If you need assistance implementing this verification, please let me know.

tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/IDFProjectDebugProcessTest.java (1)

169-177: Hardcoded hardware dependencies make test brittle.

The test hardcodes specific hardware targets (ESP32-ETHERNET-KIT [usb://1-10] and /dev/ttyUSB1 Dual RS232-HS) which will fail if:

• The board is not connected or connected to a different USB port
• The serial port enumeration differs
• Tests run on different hardware or CI environments

Consider:

1. Making these values configurable via system properties or environment variables
2. Adding runtime detection to query available targets/ports and select the first available one
3. Adding a clear skip condition with a descriptive message when required hardware is not present

Example using system properties:

private static void whenSelectLaunchTargetBoard() throws Exception
{
    String board = System.getProperty("esp.test.board", "ESP32-ETHERNET-KIT [usb://1-10]");
    selectLaunchTarget("Board:", board);
}

private static void whenSelectLaunchTargetSerialPort() throws Exception
{
    String port = System.getProperty("esp.test.serial.port", "/dev/ttyUSB1 Dual RS232-HS");
    selectLaunchTarget("Serial Port:", port);
}

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro