Skip to content

Commit 1f12aec

Browse files
missedonemissedone
authored
Merge pull request #585 from testng-team/claude/get-last-updated-issue-ExhgJ
2 parents d7abd0a + 334ad09 commit 1f12aec

File tree

1 file changed

+242
-0
lines changed

1 file changed

+242
-0
lines changed

‎DESIGN-pagination.md‎

Lines changed: 242 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,242 @@
1+
# Design: Paginated Test Result Display
2+
3+
## Problem
4+
5+
When running thousands of parameterized tests, the TestNG Eclipse plugin's UI rendering lags far behind test execution. The root cause: every single test result triggers a synchronous UI update (`postSyncRunnable` -> `Display.syncExec`) that creates SWT `TreeItem` nodes. With thousands of results, this creates thousands of synchronous round-trips to the UI thread, each allocating SWT resources and expanding tree nodes.
6+
7+
## Current Architecture
8+
9+
```
10+
Test Process --[socket]--> EclipseTestRunnerClient
11+
--> SuiteRunInfo (stores all RunInfo in List<RunInfo>)
12+
--> TestRunnerViewPart.postTestResult()
13+
--> postSyncRunnable {
14+
progressBar.step()
15+
for each tab: tab.updateTestResult(runInfo) // creates TreeItems
16+
}
17+
```
18+
19+
**Key classes:**
20+
- `RunInfo` - immutable data object for a single test result
21+
- `SuiteRunInfo` - aggregates all `RunInfo` in `List<RunInfo> m_results`
22+
- `AbstractTab` - renders tree: Suite > Test > Class > Method hierarchy
23+
- `TestRunnerViewPart` - orchestrates everything, implements listener interfaces
24+
25+
**Current bottleneck:** `AbstractTab.updateTestResult(RunInfo, boolean)` is called for every result, creating `TreeItem` widgets and expanding nodes synchronously on the UI thread.
26+
27+
## Proposed Solution: Pagination with Virtual Data Store
28+
29+
Inspired by JUnit's approach of limiting display to N tests at a time, but with navigable pages.
30+
31+
### 1. Data Storage Layer: `PagedResultStore`
32+
33+
A new class that sits between `SuiteRunInfo` (which keeps all results) and the tree tabs (which render them).
34+
35+
```
36+
PagedResultStore
37+
├── allResults: List<RunInfo> // reference to SuiteRunInfo.m_results
38+
├── filteredResults: List<RunInfo> // after applying accept + search filter
39+
├── pageSize: int // default 1000, configurable
40+
├── currentPage: int // 0-based
41+
├── getPageResults(): List<RunInfo> // returns current page slice
42+
├── getTotalPages(): int
43+
├── getTotalFilteredCount(): int
44+
├── setPage(int page): void
45+
├── nextPage() / prevPage(): void
46+
├── setFilter(Predicate<RunInfo>): void
47+
└── addResult(RunInfo): void // appends, updates filteredResults if matches
48+
```
49+
50+
**Design decisions:**
51+
- `allResults` is a shared reference to `SuiteRunInfo.m_results` (no data duplication)
52+
- `filteredResults` is lazily rebuilt only when the filter changes
53+
- Page navigation only re-renders the current page slice (rebuild tree from scratch for that page)
54+
- During a live test run, new results append to the last page; the tree is only updated if the user is viewing the last page
55+
56+
### 2. Modified AbstractTab
57+
58+
```
59+
AbstractTab (modified)
60+
├── m_store: PagedResultStore // NEW: replaces direct m_runInfos tracking
61+
├── m_runInfos: Set<RunInfo> // REMOVED (data now in store)
62+
├── updateTestResult(RunInfo, expand) // MODIFIED: delegates to store, conditionally renders
63+
├── updateTestResult(List<RunInfo>) // MODIFIED: bulk-loads into store, renders current page
64+
├── renderCurrentPage() // NEW: clears tree, renders only page slice
65+
├── updateSearchFilter(String) // MODIFIED: updates store filter, re-renders page
66+
└── [existing tree-building logic stays but only operates on page slice]
67+
```
68+
69+
**Key behavioral changes:**
70+
71+
| Scenario | Current Behavior | New Behavior |
72+
|----------|-----------------|--------------|
73+
| Result arrives during run | Creates TreeItem immediately | Adds to store; if on last page AND page not full, creates TreeItem. Otherwise, just updates page label. |
74+
| Suite finishes | Rebuilds entire tree with all results | Renders page 1 only (1000 items max) |
75+
| User changes search filter | Rebuilds tree with all matching results | Rebuilds filtered list in store, renders page 1 of filtered results |
76+
| User navigates page | N/A | Clears tree, renders requested page slice |
77+
78+
### 3. Pagination UI Controls
79+
80+
Added to `AbstractTab.createTabControl()`, below the tree widget:
81+
82+
```
83+
┌──────────────────────────────────────────────────────┐
84+
│ [Tree: Suite > Test > Class > Method...] │
85+
│ │
86+
│ │
87+
├──────────────────────────────────────────────────────┤
88+
│ [<<] [<] Page 1 of 5 (4237 results) [>] [>>] │
89+
└──────────────────────────────────────────────────────┘
90+
```
91+
92+
Components:
93+
- `Composite m_paginationBar` - horizontal bar at bottom of tree area
94+
- `Button m_firstPage, m_prevPage, m_nextPage, m_lastPage` - navigation buttons
95+
- `Label m_pageInfo` - "Page X of Y (Z results)"
96+
- Buttons disabled at boundaries (first/last page)
97+
- Bar hidden entirely when total results <= pageSize (no pagination needed)
98+
99+
### 4. Integration Points
100+
101+
#### 4.1 TestRunnerViewPart.postTestResult()
102+
103+
No change to the method signature. The existing flow remains:
104+
105+
```java
106+
postTestResult(RunInfo runInfo, boolean isSuccess) {
107+
currentSuiteRunInfo.add(runInfo); // unchanged: store in master list
108+
postSyncRunnable(() -> {
109+
progressBar.step(isSuccess); // unchanged: always update progress
110+
for (TestRunTab tab : ALL_TABS) {
111+
tab.updateTestResult(runInfo, true); // tab decides whether to render
112+
}
113+
});
114+
}
115+
```
116+
117+
The intelligence moves into `AbstractTab.updateTestResult()`:
118+
119+
```java
120+
// pseudocode
121+
void updateTestResult(RunInfo runInfo, boolean expand) {
122+
m_store.addResult(runInfo);
123+
if (m_store.isOnLastPage() && m_store.currentPageHasRoom()) {
124+
// render this single item into the tree (existing logic)
125+
renderSingleResult(runInfo, expand);
126+
}
127+
updatePaginationLabel(); // always update "Page X of Y (Z results)"
128+
}
129+
```
130+
131+
#### 4.2 AbstractTab.updateSearchFilter()
132+
133+
```java
134+
// pseudocode
135+
void updateSearchFilter(String text) {
136+
m_store.setSearchFilter(text); // rebuilds filteredResults
137+
m_store.setPage(0); // reset to first page
138+
renderCurrentPage(); // clear tree + render page slice
139+
}
140+
```
141+
142+
#### 4.3 FailureTab / SuccessTab
143+
144+
These subclasses override `acceptTestResult()`. The store's filter predicate should incorporate both:
145+
- The subclass's `acceptTestResult()` filter (pass/fail/all)
146+
- The search text filter
147+
148+
This means `PagedResultStore` needs a composite filter:
149+
150+
```java
151+
store.setFilter(runInfo -> acceptTestResult(runInfo) && matchesSearchFilter(runInfo));
152+
```
153+
154+
#### 4.4 SummaryTab
155+
156+
`SummaryTab` uses a `TableViewer` not a tree, and aggregates results differently. It should **not** be paginated (it shows one row per test, not per method). No changes needed.
157+
158+
### 5. Page Size Configuration
159+
160+
- Default: 1000 results per page
161+
- Stored in Eclipse preferences: `TestNGPluginConstants.S_PAGE_SIZE`
162+
- Accessible via Preferences > TestNG > Page Size
163+
- Reasonable range: 500-5000
164+
165+
### 6. Rendering Performance: renderCurrentPage()
166+
167+
When switching pages, the tree is fully cleared and rebuilt from the page slice:
168+
169+
```java
170+
void renderCurrentPage() {
171+
m_tree.setRedraw(false); // suppress repaints
172+
reset(); // clear all tree items and maps
173+
List<RunInfo> pageResults = m_store.getPageResults();
174+
for (RunInfo ri : pageResults) {
175+
renderSingleResult(ri, false); // don't expand individually
176+
}
177+
expandAll(); // expand all at once
178+
m_tree.setRedraw(true); // single repaint
179+
updatePaginationControls(); // update button states + label
180+
}
181+
```
182+
183+
This ensures at most `pageSize` TreeItems exist at any time, keeping SWT resource usage bounded.
184+
185+
### 7. Edge Cases
186+
187+
| Case | Handling |
188+
|------|----------|
189+
| Results < pageSize | Pagination bar hidden, behavior identical to current |
190+
| User on page 2, new results arrive | Page label updates count, tree unchanged until user navigates |
191+
| User on last page, new results arrive | If room on page, item added to tree. If page full, label updates only. |
192+
| Filter reduces results to < pageSize | Pagination bar hidden, single page shown |
193+
| Filter produces 0 results | Empty tree, label shows "0 results" |
194+
| Suite finishes (showResultsInTree) | Store resets to page 0, renders first page |
195+
| Run history switch (reset(SuiteRunInfo)) | Store gets new data source, renders page 0 |
196+
197+
### 8. Class Diagram
198+
199+
```
200+
TestRunnerViewPart
201+
│
202+
├── SuiteRunInfo (owns List<RunInfo>)
203+
│
204+
├── AbstractTab
205+
│ ├── PagedResultStore (references SuiteRunInfo's list)
206+
│ │ ├── filteredResults: List<RunInfo>
207+
│ │ ├── pageSize: int
208+
│ │ ├── currentPage: int
209+
│ │ └── filter: Predicate<RunInfo>
210+
│ │
211+
│ ├── m_tree: Tree (SWT)
212+
│ ├── m_paginationBar: Composite (NEW)
213+
│ │ ├── m_firstPage, m_prevPage: Button
214+
│ │ ├── m_pageInfo: Label
215+
│ │ └── m_nextPage, m_lastPage: Button
216+
│ │
217+
│ ├── FailureTab (acceptTestResult filters to failures)
218+
│ └── SuccessTab (acceptTestResult accepts all)
219+
│
220+
└── SummaryTab (unchanged, no pagination)
221+
```
222+
223+
### 9. Files to Modify
224+
225+
| File | Change |
226+
|------|--------|
227+
| **NEW** `PagedResultStore.java` | New class: paginated, filterable view over `List<RunInfo>` |
228+
| `AbstractTab.java` | Add `PagedResultStore`, pagination UI, modify `updateTestResult()` and `updateSearchFilter()` |
229+
| `TestRunTab.java` | No change (interface unchanged) |
230+
| `TestRunnerViewPart.java` | No change (tabs handle pagination internally) |
231+
| `SuiteRunInfo.java` | No change (data store unchanged) |
232+
| `RunInfo.java` | No change |
233+
| `TestNGPluginConstants.java` | Add `S_PAGE_SIZE` preference key |
234+
| Preference page | Add page size setting |
235+
236+
### 10. Migration Risk
237+
238+
- **Low risk**: All changes are internal to `AbstractTab` and its subclasses
239+
- **Backward compatible**: When results < pageSize, behavior is identical to current
240+
- **No protocol changes**: Data reception pipeline is untouched
241+
- **No data model changes**: `RunInfo` and `SuiteRunInfo` are unchanged
242+
- **Incremental**: Can be implemented and tested in isolation within `AbstractTab`

0 commit comments

Comments
 (0)