SWTBot test case: JTAG Flash Process verification #1300
Conversation
WalkthroughAdds a new SWTBot JUnit test for JTAG flashing, a console-verification helper in ProjectTestOperations, a Linux-only gate in an existing serial flash test, and updates CI to check out IDF release v5.4. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant J as JUnit Runner
participant T as IDFProjectJTAGFlashTest
participant F as Fixture/Helpers
participant E as Eclipse UI
participant RC as Run Configurations
participant C as Console
J->>T: beforeTestClass()
T->>F: Load Espressif environment
J->>T: Test: create/build & configure JTAG flash
T->>F: Create project "NewProjectJTAGFlashTest"
F->>E: Complete wizard & open project
T->>F: Build project via context menu
F->>E: Trigger build & wait
T->>F: Edit launch config → set Flash over = JTAG
F->>RC: Open Run Configurations, modify target board, save
T->>E: Launch via Run Configurations
E-->>C: Stream flashing logs
T->>F: verifyTheConsoleOutput("** Flashing done for partition_table/partition-table.bin")
F->>C: Wait until text appears (timeout from property)
J->>T: afterEachTest()
T->>F: Attempt cleanup (log errors if any)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested reviewers
Poem
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/IDFProjectJTAGFlashTest.java (1)
151-155: Be tolerant to alternative dialog titlesDepending on existing targets, clicking Edit may open “Manage Launch Targets” instead of “New ESP Target”. Consider waiting for either to appear to avoid timeouts.
📜 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/IDFProjectJTAGFlashTest.java(1 hunks)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/ProjectTestOperations.java(2 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/IDFProjectJTAGFlashTest.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-777)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). (3)
- GitHub Check: build
- GitHub Check: build_macos
- GitHub Check: precommit
🔇 Additional comments (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/ProjectTestOperations.java (1)
55-57: Config key added for flash wait — confirm defaults are sufficientThe new DEFAULT_FLASH_WAIT_PROPERTY is fine. Ensure your default config files define it (ms) for CI stability; otherwise the 120s fallback may be too short on slow runners.
....ui.test/src/com/espressif/idf/ui/test/executable/cases/project/IDFProjectJTAGFlashTest.java
Outdated
Show resolved
Hide resolved
....ui.test/src/com/espressif/idf/ui/test/executable/cases/project/IDFProjectJTAGFlashTest.java
Outdated
Show resolved
Hide resolved
....ui.test/src/com/espressif/idf/ui/test/executable/cases/project/IDFProjectJTAGFlashTest.java
Outdated
Show resolved
Hide resolved
....ui.test/src/com/espressif/idf/ui/test/executable/cases/project/IDFProjectJTAGFlashTest.java
Outdated
Show resolved
Hide resolved
....ui.test/src/com/espressif/idf/ui/test/executable/cases/project/IDFProjectJTAGFlashTest.java
Outdated
Show resolved
Hide resolved
...om.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/ProjectTestOperations.java
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: 0
♻️ Duplicate comments (3)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/IDFProjectJTAGFlashTest.java (3)
59-59: Fix wizard category typo to avoid wizard navigation failure"EspressIf" will not match the wizard item; use "Espressif".
- Fixture.givenNewEspressifIDFProjectIsSelected("EspressIf", "Espressif IDF Project"); + Fixture.givenNewEspressifIDFProjectIsSelected("Espressif", "Espressif IDF Project");
131-139: Make board selection robust; avoid machine-specific USB suffix and skip cleanly if board not presentSelecting by exact string with “[usb://…]” is brittle. Match by a stable prefix and gate with Assume so CI without hardware skips instead of failing.
LaunchBarTargetSelector targetSelector = new LaunchBarTargetSelector(bot); targetSelector.clickEdit(); TestWidgetWaitUtility.waitForDialogToAppear(bot, "New ESP Target", 20000); bot.comboBoxWithLabel("IDF Target").setSelection("esp32p4"); TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot); - bot.comboBoxWithLabel("Board:").setSelection("ESP32-P4 chip (via builtin USB-JTAG) [usb://2-5]"); + SWTBotCombo boardCombo = bot.comboBoxWithLabel("Board:"); + String targetPrefix = "ESP32-P4 chip (via builtin USB-JTAG)"; + for (String item : boardCombo.items()) { + if (item.startsWith(targetPrefix)) { + boardCombo.setSelection(item); + break; + } + } + Assume.assumeTrue("ESP32-P4 USB-JTAG board not found", boardCombo.selectionIndex() != -1); TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot); - bot.button("Finish").click(); + bot.waitUntil(widgetIsEnabled(bot.button("Finish")), 10000); + bot.button("Finish").click();Add missing imports:
@@ import org.junit.Test; +import org.junit.Assume; @@ +import org.eclipse.swtbot.swt.finder.widgets.SWTBotCombo;
80-80: Replace fixed sleep with deterministic waitFixed sleeps cause flakiness; wait for jobs/operations instead.
- bot.sleep(1000); + TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot);
🧹 Nitpick comments (4)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/IDFProjectJTAGFlashTest.java (4)
123-127: Wait for “OK” to be enabled after changing “Flash over”Avoid racing the dialog; wait for enablement/state to settle before clicking.
bot.cTabItem("Main").show(); bot.cTabItem("Main").setFocus(); bot.comboBoxWithLabel("Flash over:").setSelection("JTAG"); + TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot); + bot.waitUntil(widgetIsEnabled(bot.button("OK")), 10000); bot.button("OK").click();
114-116: Add a short post-Run wait for background jobsClicking Run often schedules jobs; wait to reduce timing flakiness.
bot.waitUntil(widgetIsEnabled(bot.button("Run")), 5000); bot.button("Run").click(); + TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot);
60-60: Use a unique project name to avoid workspace collisions across runsHelps parallel runs and stale-workspace scenarios.
- Fixture.givenProjectNameIs("NewProject"); + Fixture.givenProjectNameIs("JTAGFlash_" + System.currentTimeMillis());
57-57: Nit: Fix casing in test name (“JTAGFlash”)Improves readability and consistency.
-public void givenNewProjectCreatedWhenSelectJTAGflashWhenBuiltThenCheckFlashedSuccessfully() throws Exception +public void givenNewProjectCreatedWhenSelectJTAGFlashWhenBuiltThenCheckFlashedSuccessfully() throws Exception
📜 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/IDFProjectJTAGFlashTest.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/IDFProjectJTAGFlashTest.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-777)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/IDFProjectJTAGFlashTest.java (2)
107-116: LGTM: Flash action is now actually triggeredOpening Run Configurations and invoking Run on the project addresses the prior gap.
141-144: Verify on the correct console and assert a stable success tokenEnsure you’re on “ESP-IDF Console”, and use a deterministic success message observed on your setup.
private static void thenVerifyJTAGflashDone() throws Exception { - ProjectTestOperations.verifyTheConsoleOutput(bot, "** Verify OK **"); + ProjectTestOperations.viewConsole("ESP-IDF Console", bot); + ProjectTestOperations.verifyTheConsoleOutput(bot, "Flash complete"); // Adjust token to your actual success text }Please confirm the exact success text produced by JTAG flashing on your toolchain and replace "Flash complete" accordingly.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
bundles/com.espressif.idf.core/META-INF/MANIFEST.MF (1)
19-21: Remove duplicate Require-Bundle entry for org.eclipse.jgit.
org.eclipse.jgitis listed twice; keep a single occurrence.org.eclipse.debug.core;visibility:=reexport, - org.eclipse.jgit, org.eclipse.cdt.jsoncdb.core,
📜 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)
bundles/com.espressif.idf.core/META-INF/MANIFEST.MF(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_macos
- GitHub Check: build
🔇 Additional comments (1)
bundles/com.espressif.idf.core/META-INF/MANIFEST.MF (1)
59-69: Replace embedded JARs with Orbit OSGi bundles
Embedding third-party JARs on Bundle-ClassPath increases maintenance overhead and risks class-space conflicts. Instead, consume the Orbit-provided OSGi bundles for commons-io, commons-lang3, etc., and declare them via Import-Package or Require-Bundle downstream.Please verify which of these bundles (if any) are already available in your target p2/Osgi repository and update the MANIFEST accordingly.
AndriiFilippov
force-pushed
the
IEP-1627
branch
2 times, most recently
from
a9127aa to
4be6156
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (1)
66-81: Gate with assumeTrue and remove no-op else branchThis makes intent explicit, keeps reports honest (skipped vs passed), and simplifies flow.
- if (SystemUtils.IS_OS_LINUX) //temporary solution until new ESP boards arrive - { + assumeTrue("Skipping on non-Linux (temporary until new ESP boards arrive)", SystemUtils.IS_OS_LINUX); Fixture.givenNewEspressifIDFProjectIsSelected("EspressIf", "Espressif IDF Project"); Fixture.givenProjectNameIs("NewProjectFlashTest"); Fixture.whenNewProjectIsSelected(); Fixture.whenTurnOffOpenSerialMonitorAfterFlashingInLaunchConfig(); Fixture.whenProjectIsBuiltUsingContextMenu(); Fixture.whenSelectLaunchTargetSerialPort(); Fixture.whenFlashProject(); Fixture.thenVerifyFlashDoneSuccessfully(); - } - else - { - assertTrue(true); - }Optional: move the
assumeTrue(...)into@BeforeClassto avoid environment setup on unsupported OS.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
29-29: Bump to esp-idf release/v5.4: verify compatibility and consider hardening the workflow
- Please confirm the SWTBot flash assertions (console messages, CLI flags) still match IDF v5.4 to avoid brittle test failures.
- Optional: parameterize the IDF ref (env/vars) and upgrade actions (checkout v4, setup-python v5) with fetch-depth: 1 and caching to speed runs; also pin by SHA for supply-chain safety.
- Nit: in “Publish Test Reports”, ensure
path:is a scalar (e.g.,path: tests/*/*/*/TEST-*.xmlor use a block scalar with|) to avoid YAML parsing quirks.
📜 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)
.github/workflows/ci.yml(1 hunks)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java(3 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_macos
🔇 Additional comments (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (1)
9-9: LGTM on OS detection importUsing
SystemUtils.IS_OS_LINUXis fine and keeps the gate readable.
...m/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java
Show resolved
Hide resolved
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/IDFProjectJTAGFlashTest.java (2)
142-152: Make board selection resilient; avoid hard-coded USB path suffixSelecting a fixed item with “[usb://X-Y]” is machine-specific. Choose by a stable prefix instead.
SWTBotShell shell = bot.shell("New ESP Target"); - bot.comboBoxWithLabel("Board:").setSelection("ESP32-ETHERNET-KIT [usb://1-10]"); - TestWidgetWaitUtility.waitForOperationsInProgressToFinishSync(bot); + org.eclipse.swtbot.swt.finder.widgets.SWTBotCombo boardCombo = bot.comboBoxWithLabel("Board:"); + String targetPrefix = "ESP32-ETHERNET-KIT"; + for (String item : boardCombo.items()) { + if (item.startsWith(targetPrefix)) { + boardCombo.setSelection(item); + break; + } + } + TestWidgetWaitUtility.waitForOperationsInProgressToFinishSync(bot); shell.setFocus(); bot.button("Finish").click();
67-67: Probable category typo breaks wizard navigation: “EspressIf” → “Espressif”- Fixture.givenNewEspressifIDFProjectIsSelected("EspressIf", "Espressif IDF Project"); + Fixture.givenNewEspressifIDFProjectIsSelected("Espressif", "Espressif IDF Project");
🧹 Nitpick comments (7)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/IDFProjectJTAGFlashTest.java (7)
62-80: Skip non-Linux runs instead of passing triviallyUse JUnit assumptions to mark the test as skipped on non-Linux instead of unconditionally passing via assertTrue(true). This improves reporting and avoids masking issues.
Apply:
@Test public void givenNewProjectCreatedWhenSelectJTAGflashWhenBuiltThenCheckFlashedSuccessfully() throws Exception { - if (SystemUtils.IS_OS_LINUX) //temporary solution until new ESP boards arrive for Windows - { - Fixture.givenNewEspressifIDFProjectIsSelected("EspressIf", "Espressif IDF Project"); - Fixture.givenProjectNameIs("NewProject"); - Fixture.whenNewProjectIsSelected(); - Fixture.whenSelectJTAGflashInLaunchConfig(); - Fixture.whenSelectLaunchTargetBoard(); - Fixture.whenProjectIsBuiltUsingContextMenu(); - Fixture.whenFlashProject(); - Fixture.thenVerifyJTAGflashDone(); - } - else - { - assertTrue(true); - } + org.junit.Assume.assumeTrue("Linux-only until Windows boards arrive", SystemUtils.IS_OS_LINUX); + Fixture.givenNewEspressifIDFProjectIsSelected("EspressIf", "Espressif IDF Project"); + Fixture.givenProjectNameIs("NewProject"); + Fixture.whenNewProjectIsSelected(); + Fixture.whenSelectJTAGflashInLaunchConfig(); + Fixture.whenSelectLaunchTargetBoard(); + Fixture.whenProjectIsBuiltUsingContextMenu(); + Fixture.whenFlashProject(); + Fixture.thenVerifyJTAGflashDone(); }Also add (outside this hunk):
import org.junit.Assume;
136-140: Add an explicit wait after changing “Flash over” to JTAGAvoid a potential race by waiting for UI state to settle before clicking OK.
bot.cTabItem("Main").setFocus(); bot.comboBoxWithLabel("Flash over:").setSelection("JTAG"); - bot.button("OK").click(); + TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot); + bot.waitUntil(widgetIsEnabled(bot.button("OK")), 10000); + bot.button("OK").click();
89-95: Replace fixed sleep with deterministic waitAvoid bot.sleep(1000); wait for background jobs/operations to finish.
EnvSetupOperations.setupEspressifEnv(bot); - bot.sleep(1000); + TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot); ProjectTestOperations.deleteAllProjects(bot);
127-127: Increase ‘Run’ enablement wait to reduce flakinessFive seconds can be tight on slower CI. Consider 20s or a configurable timeout.
- bot.waitUntil(widgetIsEnabled(bot.button("Run")), 5000); + bot.waitUntil(widgetIsEnabled(bot.button("Run")), 20000);
31-36: Tighten JavadocMinor clarity tweak.
-/** - * Test class to test JTAG Flash Process - * - * @author Andrii Filippov - * - */ +/** + * Verifies JTAG-based flashing of an ESP-IDF project via SWTBot. + * Author: Andrii Filippov + */
131-140: Consistent naming: ‘JTAGFlash’ instead of ‘JTAGflash’Purely cosmetic, but improves readability and consistency.
- private static void whenSelectJTAGflashInLaunchConfig() throws Exception + private static void whenSelectJTAGFlashInLaunchConfig() throws ExceptionAlso update the call site (outside this hunk):
- Fixture.whenSelectJTAGflashInLaunchConfig(); + Fixture.whenSelectJTAGFlashInLaunchConfig();
154-157: Consistent naming: ‘JTAGFlash’Optional rename to match the method above.
- private static void thenVerifyJTAGflashDone() throws Exception + private static void thenVerifyJTAGFlashDone() throws ExceptionAnd update its call site (outside this hunk):
- Fixture.thenVerifyJTAGflashDone(); + Fixture.thenVerifyJTAGFlashDone();
📜 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/IDFProjectJTAGFlashTest.java(1 hunks)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java(2 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/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java
🧰 Additional context used
🧬 Code graph analysis (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/IDFProjectJTAGFlashTest.java (3)
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-785)
⏰ 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 (2)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/IDFProjectJTAGFlashTest.java (2)
49-60: Resolved JUnit hook issue — @after is valid hereSwitching from a non-static @afterclass to @after fixes the original JUnit4 problem. Cleanup now reliably runs after each test.
120-129: Good: test actually flashes and asserts console outputYou now trigger Run from “Run Configurations...” and verify a concrete “Flashing done” marker. This addresses the earlier gap.
Also applies to: 154-157
bump IDF version to 5.4 to solve board detection issue Temporary isolate test for Linux runner only
AndriiFilippov
force-pushed
the
IEP-1627
branch
from
d9dbc0d to
efef49c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/IDFProjectJTAGFlashTest.java (4)
49-60: Cleanup hook now runs — good changeSwitching from a non-static @afterclass to @after ensures the cleanup actually executes.
131-140: Wait for UI to settle after selecting "JTAG" before clicking OKAvoids race/flakiness when the combo change triggers background work.
bot.cTabItem("Main").setFocus(); bot.comboBoxWithLabel("Flash over:").setSelection("JTAG"); - bot.button("OK").click(); + TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot); + bot.waitUntil(widgetIsEnabled(bot.button("OK")), 5000); + bot.button("OK").click();
67-67: Fix wizard category typo: "EspressIf" → "Espressif"This will break the New Project wizard tree lookup.
- Fixture.givenNewEspressifIDFProjectIsSelected("EspressIf", "Espressif IDF Project"); + Fixture.givenNewEspressifIDFProjectIsSelected("Espressif", "Espressif IDF Project");
142-152: Remove hard-coded USB suffix in board selection; scope Finish to the dialogThe board entry suffix [usb://X-Y] varies per machine; hard-coding will fail on other hosts. Also click the "Finish" button from the shell’s bot.
- LaunchBarTargetSelector targetSelector = new LaunchBarTargetSelector(bot); - targetSelector.clickEdit(); - TestWidgetWaitUtility.waitForDialogToAppear(bot, "New ESP Target", 20000); - SWTBotShell shell = bot.shell("New ESP Target"); - bot.comboBoxWithLabel("Board:").setSelection("ESP32-ETHERNET-KIT [usb://1-10]"); - TestWidgetWaitUtility.waitForOperationsInProgressToFinishSync(bot); - shell.setFocus(); - bot.button("Finish").click(); + LaunchBarTargetSelector targetSelector = new LaunchBarTargetSelector(bot); + targetSelector.clickEdit(); + TestWidgetWaitUtility.waitForDialogToAppear(bot, "New ESP Target", 20000); + SWTBotShell shell = bot.shell("New ESP Target"); + org.eclipse.swtbot.swt.finder.widgets.SWTBotCombo boardCombo = bot.comboBoxWithLabel("Board:"); + for (String item : boardCombo.items()) { + if (item.startsWith("ESP32-ETHERNET-KIT")) { + boardCombo.setSelection(item); + break; + } + } + TestWidgetWaitUtility.waitForOperationsInProgressToFinishSync(bot); + shell.setFocus(); + shell.bot().button("Finish").click();
🧹 Nitpick comments (3)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/IDFProjectJTAGFlashTest.java (3)
65-80: Skip on non-Linux using JUnit Assume instead of a no-op assertCleaner semantics and shows as “skipped” rather than a misleading pass.
Add import:
+import org.junit.Assume;Change test guard:
- if (SystemUtils.IS_OS_LINUX) //temporary solution until new ESP boards arrive for Windows - { + Assume.assumeTrue("Linux-only until boards are available on other OSes", SystemUtils.IS_OS_LINUX); Fixture.givenNewEspressifIDFProjectIsSelected("EspressIf", "Espressif IDF Project"); Fixture.givenProjectNameIs("NewProject"); Fixture.whenNewProjectIsSelected(); Fixture.whenSelectJTAGflashInLaunchConfig(); Fixture.whenSelectLaunchTargetBoard(); Fixture.whenProjectIsBuiltUsingContextMenu(); Fixture.whenFlashProject(); Fixture.thenVerifyJTAGflashDone(); - } - else - { - assertTrue(true); - }
154-157: Make console assertion less brittle across IDF versionsMatch a stable token to reduce false negatives.
- ProjectTestOperations.verifyTheConsoleOutput(bot, "** Flashing done for partition_table/partition-table.bin"); + ProjectTestOperations.verifyTheConsoleOutput(bot, "Flashing done");
89-96: Replace fixed sleep with an explicit idle/operations waitMinimizes flakiness and speeds up the suite when the system is fast.
- 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 (4)
.github/workflows/ci.yml(1 hunks)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/IDFProjectJTAGFlashTest.java(1 hunks)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java(2 hunks)tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/ProjectTestOperations.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/ci.yml
- tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java
- 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/IDFProjectJTAGFlashTest.java (3)
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-785)
⏰ 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
AndriiFilippov
changed the title
|
hi @kolipakakondal , @alirana01 |
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/IDFProjectJTAGFlashTest.java (2)
67-67: Fix category typo: “EspressIf” → “Espressif” (wizard selection will fail).- Fixture.givenNewEspressifIDFProjectIsSelected("EspressIf", "Espressif IDF Project"); + Fixture.givenNewEspressifIDFProjectIsSelected("Espressif", "Espressif IDF Project");
142-152: Make board selection resilient; avoid machine-specific USB suffix.
The “[usb://X-Y]” varies per host; selecting by prefix is safer.- SWTBotShell shell = bot.shell("New ESP Target"); - bot.comboBoxWithLabel("Board:").setSelection("ESP32-ETHERNET-KIT [usb://1-10]"); - TestWidgetWaitUtility.waitForOperationsInProgressToFinishSync(bot); - shell.setFocus(); + SWTBotShell shell = bot.shell("New ESP Target"); + org.eclipse.swtbot.swt.finder.widgets.SWTBotCombo boardCombo = bot.comboBoxWithLabel("Board:"); + for (String item : boardCombo.items()) { + if (item.startsWith("ESP32-ETHERNET-KIT")) { + boardCombo.setSelection(item); + break; + } + } + TestWidgetWaitUtility.waitForOperationsInProgressToFinishSync(bot); + shell.setFocus(); bot.button("Finish").click();
🧹 Nitpick comments (5)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/IDFProjectJTAGFlashTest.java (5)
65-80: Use Assume to skip on non-Linux instead of passing the test.
Passing masks the skip; JUnit should show “skipped”.- if (SystemUtils.IS_OS_LINUX) //temporary solution until new ESP boards arrive for Windows - { + org.junit.Assume.assumeTrue("Linux-only until boards arrive", SystemUtils.IS_OS_LINUX); Fixture.givenNewEspressifIDFProjectIsSelected("EspressIf", "Espressif IDF Project"); Fixture.givenProjectNameIs("NewProjectJTAGFlashTest"); Fixture.whenNewProjectIsSelected(); Fixture.whenSelectJTAGflashInLaunchConfig(); Fixture.whenSelectLaunchTargetBoard(); Fixture.whenProjectIsBuiltUsingContextMenu(); Fixture.whenFlashProject(); Fixture.thenVerifyJTAGflashDone(); - } - else - { - assertTrue(true); - }Also add import (or use FQN as above):
+import org.junit.Assume;
131-140: Wait for UI state before closing the config dialog.
Avoid timing flakiness after selecting “JTAG”.bot.cTabItem("Main").setFocus(); bot.comboBoxWithLabel("Flash over:").setSelection("JTAG"); - bot.button("OK").click(); + TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot); + bot.waitUntil(widgetIsEnabled(bot.button("OK")), 5000); + bot.button("OK").click();
89-95: Replace fixed sleep with an explicit wait.
Avoid arbitrary delay after environment setup.- bot.sleep(1000); + TestWidgetWaitUtility.waitForOperationsInProgressToFinishAsync(bot);
62-64: Minor: fix method name casing (“JTAGFlash”).
Purely cosmetic, improves readability.- public void givenNewProjectCreatedWhenSelectJTAGflashWhenBuiltThenCheckFlashedSuccessfully() throws Exception + public void givenNewProjectCreatedWhenSelectJTAGFlashWhenBuiltThenCheckFlashedSuccessfully() throws Exception
16-19: Drop FixMethodOrder if not needed.
With a single test, ordering adds noise.-import org.junit.FixMethodOrder; -import org.junit.runners.MethodSorters; @@ -@FixMethodOrder(MethodSorters.NAME_ASCENDING)Also applies to: 38-41
📜 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/IDFProjectJTAGFlashTest.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/IDFProjectJTAGFlashTest.java (3)
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/executable/cases/project/NewEspressifIDFProjectFlashProcessTest.java (3)
SuppressWarnings(37-170)Fixture(83-169)Fixture(73-159)
⏰ 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 (3)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/IDFProjectJTAGFlashTest.java (3)
49-60: Cleanup hook looks good.
Using @after ensures environment cleanup per test; previous @afterclass issue is resolved.
113-118: Build-and-wait logic is correct.
Build via context menu, then wait for completion and idle UI — solid.
154-157: No change needed;verifyTheConsoleOutputexists and is correctly named.
Description
JTAG flash process verification tests
Fixes # (IEP-1627)
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
Tests
Chores