mlbiscoc commented on code in PR #3263:
URL: https://github.com/apache/solr/pull/3263#discussion_r2008108962
##########
solr/core/src/test/org/apache/solr/handler/component/FacetPivot2CollectionsTest.java:
##########
@@ -65,8 +65,12 @@ public class FacetPivot2CollectionsTest extends
SolrCloudTestCase {
public static void setupCluster() throws Exception {
// create and configure cluster
configureCluster(1)
- .addConfig(COLL_A, configset("different-stopwords" + File.separator +
COLL_A))
- .addConfig(COLL_B, configset("different-stopwords" + File.separator +
COLL_B))
+ .addConfig(
+ COLL_A,
+ configset("different-stopwords" +
FileSystems.getDefault().getSeparator() + COLL_A))
Review Comment:
Yes forward slashes should work fine for resolve. I changed it accordingly.
##########
solr/core/src/test/org/apache/solr/core/TestFileSystemConfigSetService.java:
##########
@@ -73,8 +73,14 @@ public void
testIgnoresFileUploadsOutsideOfConfigSetDirectory() throws IOExcepti
// surface validation errors to enable bulk uploading
final var invalidFilePaths =
List.of(
- ".." + File.separator + "escapePath",
- "foo" + File.separator + ".." + File.separator + ".." +
File.separator + "bar");
+ ".." + FileSystems.getDefault().getSeparator() + "escapePath",
+ "foo"
+ + FileSystems.getDefault().getSeparator()
+ + ".."
+ + FileSystems.getDefault().getSeparator()
+ + ".."
+ + FileSystems.getDefault().getSeparator()
+ + "bar");
for (String invalidFilePath : invalidFilePaths) {
fileSystemConfigSetService.uploadFileToConfig(configName,
invalidFilePath, testdata, true);
Review Comment:
At some point it goes through `normalizePathToOsSeparator` inside of
uploadFileToConfig that replaces the forward slashes to
FileSystems.getDefault().getSeparator(). I just removed
FileSystems.getDefault().getSeparator() and went with forward slashes in a
string.
##########
solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java:
##########
@@ -610,9 +646,16 @@ public void testSolrHomeNotReadable() throws Exception {
makeCoreProperties("core1", false, true),
homeDir.resolve("core1").resolve(CorePropertiesLocator.PROPERTIES_FILENAME));
- assumeTrue(
- "Cannot make " + homeDir + " non-readable. Test aborted.",
- homeDir.toFile().setReadable(false, false));
+ try {
+ Set<PosixFilePermission> perms = Files.getPosixFilePermissions(homeDir);
+ perms.remove(PosixFilePermission.OWNER_READ);
+ perms.remove(PosixFilePermission.GROUP_READ);
+ perms.remove(PosixFilePermission.OTHERS_READ);
+ Files.setAttribute(homeDir, "posix:permissions", perms);
+ } catch (IOException e) {
+ throw new AssumptionViolatedException(
+ "Cannot make " + homeDir + " non-readable. Test aborted.", e);
+ }
Review Comment:
Looks like on windows this wasn't possible which is why there is a
`assumeTrue` call. This will replicate that and just ignore the test on windows.
--
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]
