epugh commented on code in PR #3263:
URL: https://github.com/apache/solr/pull/3263#discussion_r1994355061
##########
solr/core/src/test/org/apache/solr/SolrInfoBeanTest.java:
##########
@@ -88,30 +90,37 @@ public void testCallMBeanInfo() throws Exception {
}
private static List<Class<?>> getClassesForPackage(String pckgname) throws
Exception {
- ArrayList<File> directories = new ArrayList<>();
+ ArrayList<Path> directories = new ArrayList<>();
ClassLoader cld = h.getCore().getResourceLoader().getClassLoader();
String path = pckgname.replace('.', '/');
Enumeration<URL> resources = cld.getResources(path);
while (resources.hasMoreElements()) {
final URI uri = resources.nextElement().toURI();
if (!"file".equalsIgnoreCase(uri.getScheme())) continue;
- final File f = new File(uri);
+ final Path f = Path.of(uri);
directories.add(f);
}
ArrayList<Class<?>> classes = new ArrayList<>();
- for (File directory : directories) {
- if (directory.exists()) {
- String[] files = directory.list();
- for (String file : files) {
- if (file.endsWith(".class")) {
- String clazzName = file.substring(0, file.length() - 6);
- // exclude Test classes that happen to be in these packages.
- // class.ForName'ing some of them can cause trouble.
- if (!clazzName.endsWith("Test") && !clazzName.startsWith("Test")) {
- classes.add(Class.forName(pckgname + '.' + clazzName));
- }
- }
+ for (Path directory : directories) {
+ if (Files.exists(directory)) {
+ try (Stream<Path> files = Files.list(directory)) {
Review Comment:
Is the try helping? Is this a try-with-resources pattern?
##########
solr/core/src/test/org/apache/solr/cli/TestSolrCLIRunExample.java:
##########
@@ -558,32 +555,34 @@ public void testInteractiveSolrCloudExample() throws
Exception {
@Test
public void testFailExecuteScript() throws Exception {
- File solrHomeDir = new File(ExternalPaths.SERVER_HOME);
- if (!solrHomeDir.isDirectory())
- fail(solrHomeDir.getAbsolutePath() + " not found and is required to run
this test!");
+ Path solrHomeDir = Path.of(ExternalPaths.SERVER_HOME);
+ if (!Files.isDirectory(solrHomeDir))
+ fail(solrHomeDir.toAbsolutePath() + " not found and is required to run
this test!");
- Path tmpDir = createTempDir();
- File solrExampleDir = tmpDir.toFile();
- File solrServerDir = solrHomeDir.getParentFile();
+ Path solrExampleDir = createTempDir();
+ Path solrServerDir = solrHomeDir.getParent();
// need a port to start the example server on
int bindPort = -1;
try (ServerSocket socket = new ServerSocket(0)) {
bindPort = socket.getLocalPort();
}
- File toExecute = new File(tmpDir.toString(), "failExecuteScript");
- assertTrue(
- "Should have been able to create file '" + toExecute.getAbsolutePath()
+ "' ",
- toExecute.createNewFile());
+ Path toExecute = Path.of(solrExampleDir.toString(), "failExecuteScript");
+ try {
+ Files.createFile(toExecute);
+ } catch (IOException e) {
+ throw new IOException(
+ "Should have been able to create file '" +
toExecute.toAbsolutePath() + "' ");
+ }
String[] toolArgs =
new String[] {
"-e", "techproducts",
- "--server-dir", solrServerDir.getAbsolutePath(),
- "--example-dir", solrExampleDir.getAbsolutePath(),
+ "--server-dir", solrServerDir.toAbsolutePath().toString(),
Review Comment:
Shame that this requires all these `.toString`.. Is there a smarter way of
handling this toolArgs? `var toolArgs`? And then internally we call
.toString on anything passed in?????
##########
solr/core/src/test/org/apache/solr/cli/StreamToolTest.java:
##########
@@ -185,11 +183,11 @@ public void testReadStream() throws Exception {
@Test
@SuppressWarnings({"unchecked", "rawtypes"})
public void testLocalCatStream() throws Exception {
- File localFile = File.createTempFile("topLevel1", ".txt");
- populateFileWithData(localFile.toPath());
+ Path localFile = Files.createTempFile("topLevel1", ".txt");
+ populateFileWithData(localFile);
StreamTool.LocalCatStream catStream =
- new StreamTool.LocalCatStream(localFile.getAbsolutePath(), -1);
+ new StreamTool.LocalCatStream(localFile.toAbsolutePath().toString(),
-1);
Review Comment:
Should we just pass in a path here?
##########
solr/core/src/test/org/apache/solr/SolrInfoBeanTest.java:
##########
@@ -88,30 +90,37 @@ public void testCallMBeanInfo() throws Exception {
}
private static List<Class<?>> getClassesForPackage(String pckgname) throws
Exception {
- ArrayList<File> directories = new ArrayList<>();
+ ArrayList<Path> directories = new ArrayList<>();
ClassLoader cld = h.getCore().getResourceLoader().getClassLoader();
String path = pckgname.replace('.', '/');
Enumeration<URL> resources = cld.getResources(path);
while (resources.hasMoreElements()) {
final URI uri = resources.nextElement().toURI();
if (!"file".equalsIgnoreCase(uri.getScheme())) continue;
- final File f = new File(uri);
+ final Path f = Path.of(uri);
Review Comment:
`path` instead of `f`?
##########
solr/core/src/test/org/apache/solr/cli/SolrProcessManagerTest.java:
##########
@@ -84,9 +83,11 @@ private static int findAvailablePort() throws IOException {
private static Pair<Integer, Process> createProcess(int port, boolean https)
throws IOException {
// Get the path to the java executable from the current JVM
String classPath =
-
Arrays.stream(System.getProperty("java.class.path").split(File.pathSeparator))
+ Arrays.stream(
+
System.getProperty("java.class.path").split(System.getProperty("path.separator")))
Review Comment:
I'm suprised we don't have sort of nicer way of doing this than
`System.getProperty`.. Is this common enough that we should have a helper?
##########
solr/core/src/test/org/apache/solr/cli/SolrProcessManagerTest.java:
##########
@@ -84,9 +83,11 @@ private static int findAvailablePort() throws IOException {
private static Pair<Integer, Process> createProcess(int port, boolean https)
throws IOException {
// Get the path to the java executable from the current JVM
String classPath =
-
Arrays.stream(System.getProperty("java.class.path").split(File.pathSeparator))
+ Arrays.stream(
+
System.getProperty("java.class.path").split(System.getProperty("path.separator")))
Review Comment:
Is "FileSystems.getDefault().getSeparator();" a possiblity?
--
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: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
