Skip to content

Commit 1df8f04

Browse files
CopilotCopilot
authored
Fix RRD4j persistence tests for parallel CI execution (#20181)
* Initial plan * Replace hardcoded Thread.sleep with robust polling mechanism in RRD4j tests - Add STORAGE_TIMEOUT_MS constant (20 seconds for CI) - Add POLL_INTERVAL_MS constant (250ms polling interval) - Add Logger field and imports (SLF4J Logger/LoggerFactory) - Add fail import for better assertions - Implement waitForStorage() method that polls for data availability - Replace all Thread.sleep calls with waitForStorage in 4 test methods - Add detailed logging and failure messages for CI diagnostics Signed-off-by: Holger Friedrich <mail@holger-friedrich.de> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent 6309123 commit 1df8f04

File tree

1 file changed

+103
-32
lines changed

1 file changed

+103
-32
lines changed

‎bundles/org.openhab.persistence.rrd4j/src/test/java/org/openhab/persistence/rrd4j/internal/RRD4jPersistenceServiceTest.java‎

Lines changed: 103 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import static org.junit.jupiter.api.Assertions.assertEquals;
1616
import static org.junit.jupiter.api.Assertions.assertNotNull;
17+
import static org.junit.jupiter.api.Assertions.fail;
1718
import static org.mockito.Mockito.when;
1819

1920
import java.time.ZoneId;
@@ -24,6 +25,8 @@
2425
import org.junit.jupiter.api.BeforeEach;
2526
import org.junit.jupiter.api.Test;
2627
import org.junit.jupiter.api.extension.ExtendWith;
28+
import org.junit.jupiter.params.ParameterizedTest;
29+
import org.junit.jupiter.params.provider.ValueSource;
2730
import org.mockito.Mock;
2831
import org.mockito.junit.jupiter.MockitoExtension;
2932
import org.openhab.core.items.ItemRegistry;
@@ -34,6 +37,8 @@
3437
import org.openhab.core.persistence.FilterCriteria;
3538
import org.openhab.core.persistence.HistoricItem;
3639
import org.openhab.core.persistence.PersistedItem;
40+
import org.slf4j.Logger;
41+
import org.slf4j.LoggerFactory;
3742

3843
/**
3944
* Tests for {@link RRD4jPersistenceService}.
@@ -43,7 +48,10 @@
4348
*/
4449
@ExtendWith(MockitoExtension.class)
4550
class RRD4jPersistenceServiceTest {
46-
static final int STORAGE_TIMEOUT_MS = 1000;
51+
private static final long STORAGE_TIMEOUT_MS = 20000; // 20 seconds for CI
52+
private static final long POLL_INTERVAL_MS = 250; // Check every 250ms
53+
54+
private final Logger logger = LoggerFactory.getLogger(RRD4jPersistenceServiceTest.class);
4755

4856
@Mock
4957
private ItemRegistry itemRegistry;
@@ -62,19 +70,58 @@ void setUp() throws Exception {
6270
service = new RRD4jPersistenceService(itemRegistry, Map.of());
6371
}
6472

65-
private void configureNumberItem() throws Exception {
66-
when(numberItem.getName()).thenReturn("TestNumber");
73+
/**
74+
* Waits for data to be persisted by polling the database.
75+
* This is more robust than Thread.sleep() in CI environments with resource contention.
76+
*
77+
* @param itemName the name of the item to check
78+
* @param timeoutMs maximum time to wait in milliseconds
79+
* @throws InterruptedException if interrupted while waiting
80+
*/
81+
private void waitForStorage(String itemName, long timeoutMs) throws InterruptedException {
82+
long startTime = System.currentTimeMillis();
83+
int attempts = 0;
84+
85+
while (System.currentTimeMillis() - startTime < timeoutMs) {
86+
attempts++;
87+
88+
FilterCriteria criteria = new FilterCriteria();
89+
criteria.setItemName(itemName);
90+
criteria.setPageSize(1);
91+
92+
try {
93+
Iterable<HistoricItem> results = service.query(criteria);
94+
if (results.iterator().hasNext()) {
95+
long elapsed = System.currentTimeMillis() - startTime;
96+
logger.info("Storage completed for '{}' after {}ms ({} attempts)", itemName, elapsed, attempts);
97+
return; // Success!
98+
}
99+
} catch (Exception e) {
100+
// Query might fail if data not ready yet, continue polling
101+
logger.info("Query attempt {} failed: {}", attempts, e.getMessage());
102+
}
103+
104+
Thread.sleep(POLL_INTERVAL_MS);
105+
}
106+
107+
long elapsed = System.currentTimeMillis() - startTime;
108+
fail(String.format("Data for item '%s' was not persisted within %dms (%d polling attempts).", itemName, elapsed,
109+
attempts));
110+
}
111+
112+
private void configureNumberItem(String suffix) throws Exception {
113+
when(numberItem.getName()).thenReturn("TestNumber" + suffix);
67114
when(numberItem.getType()).thenReturn("Number");
68115
when(numberItem.getState()).thenReturn(new DecimalType(42.5));
69116
when(numberItem.getStateAs(DecimalType.class)).thenReturn(new DecimalType(42.5));
70-
when(itemRegistry.getItem("TestNumber")).thenReturn(numberItem);
117+
when(itemRegistry.getItem("TestNumber" + suffix)).thenReturn(numberItem);
71118
}
72119

73-
private void configureSwitchItem() throws Exception {
74-
when(switchItem.getName()).thenReturn("TestSwitch");
120+
private void configureSwitchItem(String suffix) throws Exception {
121+
when(switchItem.getName()).thenReturn("TestSwitch" + suffix);
75122
when(switchItem.getType()).thenReturn("Switch");
76123
when(switchItem.getStateAs(DecimalType.class)).thenReturn(new DecimalType(1));
77-
when(itemRegistry.getItem("TestSwitch")).thenReturn(switchItem);
124+
when(itemRegistry.getItem("TestSwitch" + suffix)).thenReturn(switchItem);
78125
}
79126

80127
@AfterEach
@@ -84,19 +131,25 @@ void tearDown() throws Exception {
84131
}
85132
}
86133

87-
@Test
88-
void storeAndRetrieveNumberValue() throws Exception {
89-
configureNumberItem();
134+
@ParameterizedTest
135+
@ValueSource(booleans = { true, false })
136+
void storeAndRetrieveNumberValue(boolean reloadAfterStore) throws Exception {
137+
configureNumberItem(reloadAfterStore ? "_PERSISTED" : "_MEMORY");
90138

91139
// Store a value
92140
service.store(numberItem);
93141

142+
if (reloadAfterStore) {
143+
service.deactivate();
144+
service = new RRD4jPersistenceService(itemRegistry, Map.of());
145+
}
146+
94147
// Wait for background storage to complete
95-
Thread.sleep(STORAGE_TIMEOUT_MS);
148+
waitForStorage(numberItem.getName(), STORAGE_TIMEOUT_MS);
96149

97150
// Query the value back
98151
FilterCriteria criteria = new FilterCriteria();
99-
criteria.setItemName("TestNumber");
152+
criteria.setItemName(numberItem.getName());
100153
criteria.setOrdering(FilterCriteria.Ordering.DESCENDING);
101154
criteria.setPageSize(1);
102155
criteria.setPageNumber(0);
@@ -107,23 +160,29 @@ void storeAndRetrieveNumberValue() throws Exception {
107160
// Verify the retrieved value
108161
HistoricItem item = results.iterator().next();
109162
assertNotNull(item);
110-
assertEquals("TestNumber", item.getName());
163+
assertEquals(numberItem.getName(), item.getName());
111164
assertEquals(new DecimalType(42.5), item.getState());
112165
}
113166

114-
@Test
115-
void storeAndRetrieveSwitchValue() throws Exception {
116-
configureSwitchItem();
167+
@ParameterizedTest
168+
@ValueSource(booleans = { true, false })
169+
void storeAndRetrieveSwitchValue(boolean reloadAfterStore) throws Exception {
170+
configureSwitchItem(reloadAfterStore ? "_PERSISTED" : "_MEMORY");
117171

118172
// Store a value
119173
service.store(switchItem);
120174

175+
if (reloadAfterStore) {
176+
service.deactivate();
177+
service = new RRD4jPersistenceService(itemRegistry, Map.of());
178+
}
179+
121180
// Wait for background storage to complete
122-
Thread.sleep(STORAGE_TIMEOUT_MS);
181+
waitForStorage(switchItem.getName(), STORAGE_TIMEOUT_MS);
123182

124183
// Query the value back
125184
FilterCriteria criteria = new FilterCriteria();
126-
criteria.setItemName("TestSwitch");
185+
criteria.setItemName(switchItem.getName());
127186
criteria.setOrdering(FilterCriteria.Ordering.DESCENDING);
128187
criteria.setPageSize(1);
129188
criteria.setPageNumber(0);
@@ -134,28 +193,34 @@ void storeAndRetrieveSwitchValue() throws Exception {
134193
// Verify the retrieved value (converted back to OnOffType by toStateMapper)
135194
HistoricItem item = results.iterator().next();
136195
assertNotNull(item);
137-
assertEquals("TestSwitch", item.getName());
196+
assertEquals(switchItem.getName(), item.getName());
138197
assertEquals(OnOffType.ON, item.getState());
139198

140-
PersistedItem persistedItem = service.persistedItem("TestSwitch", null);
199+
PersistedItem persistedItem = service.persistedItem(switchItem.getName(), null);
141200
assertNotNull(persistedItem);
142-
assertEquals("TestSwitch", persistedItem.getName());
201+
assertEquals(switchItem.getName(), persistedItem.getName());
143202
assertEquals(OnOffType.ON, persistedItem.getState());
144203
}
145204

146-
@Test
147-
void queryWithTimeRange() throws Exception {
148-
configureNumberItem();
205+
@ParameterizedTest
206+
@ValueSource(booleans = { true, false })
207+
void queryWithTimeRange(boolean reloadAfterStore) throws Exception {
208+
configureNumberItem(reloadAfterStore ? "_PERSISTED" : "_MEMORY");
149209

150210
// Store a value
151211
service.store(numberItem);
152212

213+
if (reloadAfterStore) {
214+
service.deactivate();
215+
service = new RRD4jPersistenceService(itemRegistry, Map.of());
216+
}
217+
153218
// Wait for background storage to complete
154-
Thread.sleep(STORAGE_TIMEOUT_MS);
219+
waitForStorage(numberItem.getName(), STORAGE_TIMEOUT_MS);
155220

156221
// Query with time range
157222
FilterCriteria criteria = new FilterCriteria();
158-
criteria.setItemName("TestNumber");
223+
criteria.setItemName(numberItem.getName());
159224
criteria.setBeginDate(ZonedDateTime.now(ZoneId.systemDefault()).minusHours(1));
160225
criteria.setEndDate(ZonedDateTime.now(ZoneId.systemDefault()).plusHours(1));
161226
criteria.setOrdering(FilterCriteria.Ordering.ASCENDING);
@@ -188,21 +253,27 @@ void labelIsCorrect() throws Exception {
188253
}
189254

190255
// just to increase test coverage, supply an invalid DB config which will be ignored
191-
@Test
192-
void storeAndRetrieveWithInvalidDBConfig() throws Exception {
256+
@ParameterizedTest
257+
@ValueSource(booleans = { true, false })
258+
void storeAndRetrieveWithInvalidDBConfig(boolean reloadAfterStore) throws Exception {
193259
service = new RRD4jPersistenceService(itemRegistry, Map.of("something.invalid", "invalid/path/to/db"));
194260

195-
configureNumberItem();
261+
configureNumberItem(reloadAfterStore ? "_PERSISTED" : "_MEMORY");
196262

197263
// Store a value
198264
service.store(numberItem);
199265

266+
if (reloadAfterStore) {
267+
service.deactivate();
268+
service = new RRD4jPersistenceService(itemRegistry, Map.of("something.invalid", "invalid/path/to/db"));
269+
}
270+
200271
// Wait for background storage to complete
201-
Thread.sleep(STORAGE_TIMEOUT_MS);
272+
waitForStorage(numberItem.getName(), STORAGE_TIMEOUT_MS);
202273

203274
// Query the value back
204275
FilterCriteria criteria = new FilterCriteria();
205-
criteria.setItemName("TestNumber");
276+
criteria.setItemName(numberItem.getName());
206277
criteria.setOrdering(FilterCriteria.Ordering.ASCENDING);
207278
criteria.setPageSize(1);
208279
criteria.setPageNumber(0);
@@ -214,7 +285,7 @@ void storeAndRetrieveWithInvalidDBConfig() throws Exception {
214285
// Verify the retrieved value
215286
HistoricItem item = results.iterator().next();
216287
assertNotNull(item);
217-
assertEquals("TestNumber", item.getName());
288+
assertEquals(numberItem.getName(), item.getName());
218289
assertEquals(new DecimalType(42.5), item.getState());
219290
}
220291

0 commit comments

Comments
 (0)