Copilot commented on code in PR #8151:
URL: https://github.com/apache/incubator-seata/pull/8151#discussion_r3567743330
##########
config/seata-config-core/src/test/java/org/apache/seata/config/FileConfigurationTest.java:
##########
@@ -471,4 +471,48 @@ void testGetConfigFromEnvironmentVariable() {
String path = fileConfig.getConfigFromSys("PATH");
Assertions.assertNotNull(path);
}
+
+ @Test
+ void shouldDelegateFileBackedReadsThroughConfigurationFactory() {
+ String dataId = "service.disableGlobalTransaction";
+ String previousValue = System.getProperty(dataId);
+ try {
+ System.clearProperty(dataId);
+ ConfigurationFactory.reload();
+ Configuration fileConfig = ConfigurationFactory.getInstance();
+
+ Assertions.assertEquals("127.0.0.1:8091",
fileConfig.getConfig("service.default.grouplist"));
+ Assertions.assertFalse(fileConfig.getBoolean(dataId));
+ } finally {
+ if (previousValue == null) {
+ System.clearProperty(dataId);
+ } else {
+ System.setProperty(dataId, previousValue);
+ }
+ ConfigurationFactory.reload();
+ }
+ }
+
+ @Test
+ void
shouldReturnDefaultsForMissingConfigurationThroughConfigurationFactory() {
+ Configuration fileConfig = ConfigurationFactory.getInstance();
+
+ Assertions.assertNull(fileConfig.getConfig("adopted.missing.key"));
+ Assertions.assertEquals("fallback",
fileConfig.getLatestConfig("adopted.missing.key", "fallback", 1000L));
+ Assertions.assertEquals(39, fileConfig.getInt("adopted.missing.int",
39));
+ Assertions.assertEquals((short) 2,
fileConfig.getShort("adopted.missing.short", (short) 2));
+ }
+
+ @Test
+ void shouldReportMutationOperationOutcomeThroughConfigurationFactory() {
+ Configuration fileConfig = ConfigurationFactory.getInstance();
+
+ Assertions.assertTrue(fileConfig.putConfig("adopted.put.key", "value",
1000L));
+
Assertions.assertTrue(fileConfig.putConfigIfAbsent("adopted.put-if-absent.key",
"value", 1000L));
+ Assertions.assertTrue(fileConfig.removeConfig("adopted.remove.key",
1000L));
Review Comment:
The assertions expecting `putConfig` / `putConfigIfAbsent` / `removeConfig`
to return `true` within 1000ms can be flaky because these operations run
asynchronously and `ConfigFuture.get(timeout)` returns `false` on timeout.
Consider using a larger timeout (e.g., >= the default 5s) for the “success”
path to avoid intermittent CI failures under load; keep the negative timeout
cases for the timeout-path regression.
##########
config/seata-config-core/src/test/java/org/apache/seata/config/FileConfigurationTest.java:
##########
@@ -471,4 +471,48 @@ void testGetConfigFromEnvironmentVariable() {
String path = fileConfig.getConfigFromSys("PATH");
Assertions.assertNotNull(path);
}
+
+ @Test
+ void shouldDelegateFileBackedReadsThroughConfigurationFactory() {
+ String dataId = "service.disableGlobalTransaction";
+ String previousValue = System.getProperty(dataId);
+ try {
+ System.clearProperty(dataId);
+ ConfigurationFactory.reload();
+ Configuration fileConfig = ConfigurationFactory.getInstance();
+
+ Assertions.assertEquals("127.0.0.1:8091",
fileConfig.getConfig("service.default.grouplist"));
+ Assertions.assertFalse(fileConfig.getBoolean(dataId));
+ } finally {
+ if (previousValue == null) {
+ System.clearProperty(dataId);
+ } else {
+ System.setProperty(dataId, previousValue);
+ }
+ ConfigurationFactory.reload();
+ }
Review Comment:
This test tries to assert values coming from `file.conf`, but it only
clears/restores the system property for `service.disableGlobalTransaction`. If
`service.default.grouplist` is set as a system property in the test JVM/CI,
`getConfig("service.default.grouplist")` will return the system property (via
`getConfigFromSys`) instead of the file-backed value, making the test
order-/environment-dependent. Clear and restore the `service.default.grouplist`
system property in the same way as `service.disableGlobalTransaction` before
reloading the factory.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
