SOLR-18057: Prefer Path.resolve over Path.of for derived paths#4174
Conversation
epugh
left a comment
There was a problem hiding this comment.
This looks reasonable. Would it be possible to use our various validation tools to prevent this from happening in the future?
|
Tooling: There's nothing to ban. I wish there was a PR/review mechanism that would basically point out usages of |
|
I was thinking that one of ecj or error prone or forbidden APIs checkers would keep this from coming back. |
|
I dug around a bit and the only way to enforce this would be to write a custom BugChecker in Error Prone which probably isn't worth it. |
|
I ran the tests and they all passed local... |
Co-authored-by: Kamlendra <kamlendrachauhan21@gmail.com>
This PR addresses SOLR-18057 by eliminating unnecessary Path→String→Path conversions and using
Path.resolve(...)when the base is already a Path (starting with BinaryFieldTest noted in the issue).No intended behavior change; refactor only.
What changed
TestBinaryFieldto useTEST_HOME().resolve("collection1").resolve("conf")andresolve(...)for file paths.Path:TestRawTransformerTestCustomCorePropertiesTestCoreDiscoveryTestLazyCoresCoreAdminCreateDiscoverTestCoreAdminHandlerTestReplicationTestHelperTestSystemIdResolverValidation
:solr:core:test --tests org.apache.solr.schema.TestBinaryField:solr:core:test --tests org.apache.solr.util.TestSystemIdResolver:solr:core:test --tests org.apache.solr.response.TestRawTransformer:solr:core:test --tests org.apache.solr.TestCustomCoreProperties./gradlew checkwas attempted and hit unrelated environment/flaky issues (CloudSolrClientCacheTestflake and RAT task wiring check in this local environment).