Copilot commented on code in PR #3905:
URL: https://github.com/apache/solr/pull/3905#discussion_r2573633693
##########
solr/core/src/test/org/apache/solr/TestSolrCoreProperties.java:
##########
@@ -61,12 +62,17 @@ public static void beforeTest() throws Exception {
Properties p = new Properties();
p.setProperty("foo.foo1", "f1");
p.setProperty("foo.foo2", "f2");
- try (Writer fos =
- Files.newBufferedWriter(confDir.resolve("solrcore.properties"),
StandardCharsets.UTF_8)) {
+ var coreCustomProperties =
confDir.resolve("core_custom_properties.properties");
+ try (Writer fos = Files.newBufferedWriter(coreCustomProperties,
StandardCharsets.UTF_8)) {
p.store(fos, null);
}
- Files.createFile(collDir.resolve("core.properties"));
+ Properties coreProperties = new Properties();
+ coreProperties.setProperty(CoreDescriptor.CORE_PROPERTIES,
coreCustomProperties.toString());
Review Comment:
Using an absolute path (via `toString()`) instead of a relative path is
fragile and inconsistent with the documented behavior. According to the javadoc
in `CoreDescriptor.loadExtraProperties()`, "File paths are taken as read from
the core's instance directory if they are not absolute."
Consider using a relative path instead:
```java
coreProperties.setProperty(CoreDescriptor.CORE_PROPERTIES,
"conf/core_custom_properties.properties");
```
This would be more consistent with the expected usage pattern and clearer in
intent.
```suggestion
coreProperties.setProperty(CoreDescriptor.CORE_PROPERTIES,
"conf/core_custom_properties.properties");
```
--
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]
