test: stabilize sdkconfig partition offset UI test on Linux#1477
Conversation
|
Warning Review limit reached
More reviews will be available in 13 minutes and 46 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR refactors partition table offset validation logic in the SDK configuration test by extracting inline UI interactions into four reusable helper methods. Existing test flows are updated to use these helpers, improving test maintainability and reducing duplication across verification and save-flow validation steps. ChangesSDK Config Test Refactoring
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectSDKconfigTest.java (1)
175-176: ⚡ Quick winExtract the offset field label into a single constant.
"Offset of partition table (hex)"is repeated across three helpers. One UI label change will break multiple call sites independently.Proposed refactor
+ private static final String PARTITION_OFFSET_LABEL = "Offset of partition table (hex)"; ... - SWTBotText offsetField = bot.textWithLabel("Offset of partition table (hex)"); + SWTBotText offsetField = bot.textWithLabel(PARTITION_OFFSET_LABEL); ... - return bot.textWithLabel("Offset of partition table (hex)").getText() + return bot.textWithLabel(PARTITION_OFFSET_LABEL).getText() .equalsIgnoreCase(hexValue); ... - String actual = bot.textWithLabel("Offset of partition table (hex)").getText(); + String actual = bot.textWithLabel(PARTITION_OFFSET_LABEL).getText();Also applies to: 197-198, 211-213
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectSDKconfigTest.java` around lines 175 - 176, Extract the repeated UI label into a single class-level constant in NewEspressifIDFProjectSDKconfigTest (e.g., private static final String OFFSET_PARTITION_TABLE_LABEL = "Offset of partition table (hex)"); then replace the literal string used in all bot.textWithLabel(...) calls (the occurrences that create SWTBotText offsetField and similar helper calls) with this constant so a single change to the label updates every call site.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectSDKconfigTest.java`:
- Around line 173-188: In setPartitionTableOffset, always use user-like typing
instead of setText: find the SWTBotText offsetField (variable offsetField) and
replace the primary write path (offsetField.setText(hexValue)) with
offsetField.selectAll() followed by offsetField.typeText(hexValue), keeping the
existing focus/selectAll and bot.sleep calls and leaving the fallback
check/remove of setText entirely; ensure you still retain the existing getText()
check only if you want to verify, but do not rely on setText to persist the
value—use selectAll()+typeText(hexValue) as the canonical write operation for
offsetField in the setPartitionTableOffset method.
---
Nitpick comments:
In
`@tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectSDKconfigTest.java`:
- Around line 175-176: Extract the repeated UI label into a single class-level
constant in NewEspressifIDFProjectSDKconfigTest (e.g., private static final
String OFFSET_PARTITION_TABLE_LABEL = "Offset of partition table (hex)"); then
replace the literal string used in all bot.textWithLabel(...) calls (the
occurrences that create SWTBotText offsetField and similar helper calls) with
this constant so a single change to the label updates every call site.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 55b17fbc-ddc6-4ebd-8061-a1ffe8775a1e
📒 Files selected for processing (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectSDKconfigTest.java
| private static void setPartitionTableOffset(String hexValue) | ||
| { | ||
| SWTBotText offsetField = bot.textWithLabel("Offset of partition table (hex)"); | ||
| offsetField.setFocus(); | ||
| offsetField.selectAll(); | ||
| offsetField.setText(hexValue); | ||
| bot.sleep(500); | ||
|
|
||
| if (!offsetField.getText().equalsIgnoreCase(hexValue)) | ||
| { | ||
| offsetField.setFocus(); | ||
| offsetField.selectAll(); | ||
| offsetField.typeText(hexValue); | ||
| bot.sleep(500); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In SWTBot on Linux/GTK, does SWTBotText#setText(...) reliably trigger ModifyListeners for text fields, and is selectAll()+typeText(...) the recommended approach for listener-driven UI updates?
💡 Result:
In SWTBot, the method SWTBotText#setText(String) does not reliably trigger ModifyListeners on all platforms, including Linux/GTK [1]. Because it often updates the widget's text property directly (frequently using syncExec to set the widget value), it bypasses the native UI events that would typically fire a ModifyListener [1]. Using selectAll followed by typeText(String) is the recommended approach if your goal is to trigger listener-driven UI updates [2]. This sequence mimics actual user interaction by selecting the existing text and typing the new input, which forces the underlying SWT Text widget to process standard key events and trigger associated ModifyListeners as a user would [2]. While typeText is more realistic for testing UI behavior triggered by input events, it can be slower and occasionally inconsistent depending on focus or platform-specific GTK behavior [3][2]. If you do not require ModifyListener execution, setText remains the faster, more direct way to populate fields.
Citations:
- 1: https://bugs.eclipse.org/bugs/show_bug.cgi?id=287210
- 2: https://bugs.eclipse.org/bugs/show_bug.cgi?id=273287
- 3: https://bugs.eclipse.org/bugs/show_bug.cgi?id=81695
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectSDKconfigTest.java"
nl -ba "$FILE" | sed -n '150,210p'Repository: espressif/idf-eclipse-plugin
Length of output: 115
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectSDKconfigTest.java"
awk 'NR>=150 && NR<=210 {printf "%d\t%s\n", NR, $0}' "$FILE"Repository: espressif/idf-eclipse-plugin
Length of output: 2165
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectSDKconfigTest.java"
rg -n "setPartitionTableOffset\\(|waitForPartitionTableOffset\\(|click\\(\\)|Save|Menu Config|TestWidgetWaitUtility\\.waitForOperationsInProgressToFinishSync" "$FILE" || trueRepository: espressif/idf-eclipse-plugin
Length of output: 1442
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectSDKconfigTest.java"
awk 'NR>=80 && NR<=170 {printf "%d\t%s\n", NR, $0}' "$FILE"
echo "----"
awk 'NR>=200 && NR<=270 {printf "%d\t%s\n", NR, $0}' "$FILE"Repository: espressif/idf-eclipse-plugin
Length of output: 6487
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectSDKconfigTest.java"
awk 'NR>=80 && NR<=170 {printf "%d\t%s\n", NR, $0}' "$FILE"
echo "----"
awk 'NR>=200 && NR<=270 {printf "%d\t%s\n", NR, $0}' "$FILE"Repository: espressif/idf-eclipse-plugin
Length of output: 6487
Use user-like typing (selectAll + typeText) as the primary write path in setPartitionTableOffset
setPartitionTableOffset uses offsetField.setText(hexValue) first and only falls back to typeText when the displayed text doesn’t match. On SWTBot Linux/GTK, setText(...) can bypass native input/ModifyListener behavior, so the underlying persisted value can stay stale even when offsetField.getText() already equals the expected hex—leading to intermittent failures in the later save/reopen assertions. Prefer always doing selectAll() + typeText(hexValue) for this field.
[done]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/executable/cases/project/NewEspressifIDFProjectSDKconfigTest.java`
around lines 173 - 188, In setPartitionTableOffset, always use user-like typing
instead of setText: find the SWTBotText offsetField (variable offsetField) and
replace the primary write path (offsetField.setText(hexValue)) with
offsetField.selectAll() followed by offsetField.typeText(hexValue), keeping the
existing focus/selectAll and bot.sleep calls and leaving the fallback
check/remove of setText entirely; ensure you still retain the existing getText()
check only if you want to verify, but do not rely on setText to persist the
value—use selectAll()+typeText(hexValue) as the canonical write operation for
offsetField in the setPartitionTableOffset method.
Wait for sdkconfig save to complete and use reliable hex field input so partition table offset changes persist on the self-hosted Linux runner. Co-authored-by: Cursor <cursoragent@cursor.com>
kolipakakondal
force-pushed
the
fix_build_linux
branch
from
d7b6e57 to
b1875fe
Compare
1 participant
Wait for sdkconfig save to complete and use reliable hex field input so partition table offset changes persist on the self-hosted Linux runner.
Description
What the test does
NewEspressifIDFProjectSDKconfigTest (failure at line 61 → thenCheckChangesAreSaved()):
Root cause (Linux GTK timing)
On Linux/GTK, SWTBot setText() on sdkconfig hex fields can be unreliable:
This matches recent GTK sdkconfig editor layout fixes (f0c77e0, 8ff7e41) on the Eclipse 2026-03 platform used in CI.
Updated NewEspressifIDFProjectSDKconfigTest.java to be more reliable on Linux:
setPartitionTableOffset() — setFocus() → selectAll() → setText(), with typeText() fallback
waitForPartitionTableOffset() — polls until the UI shows the expected value
waitForOperationsInProgressToFinishSync() after save — ensures sdkconfig is written before rebuild/reopen
assertEquals with actual vs expected — clearer CI failure output
Fixes # https://github.com/espressif/idf-eclipse-plugin/actions/runs/27190770202/job/80270182500?pr=1476
Type of change
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