kevinrr888 commented on code in PR #5547:
URL: https://github.com/apache/accumulo/pull/5547#discussion_r2121310630
##########
test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java:
##########
@@ -324,17 +313,19 @@ public void testImportExportOperationsAudits() throws
AccumuloSecurityException,
// Prepare to export the table
Path confDirPath = getCluster().getConfig().getDir().toPath();
- File exportDir = confDirPath.resolve("export").toFile();
- File exportDirBulk = confDirPath.resolve("export_bulk").toFile();
- assertTrue(exportDirBulk.mkdir() || exportDirBulk.isDirectory());
+ Path exportDir = confDirPath.resolve("export");
+ Path exportDirBulk = confDirPath.resolve("export_bulk");
+ if (!Files.isDirectory(exportDirBulk)) {
+ Files.createDirectories(exportDirBulk);
+ }
Review Comment:
could add similar assertion: `assertTrue(Files.isDirectory(dir))` or move to
util method I mentioned earlier
##########
test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java:
##########
@@ -70,16 +72,18 @@ public void createSelfSigned() throws Exception {
@Test
public void createPublicSelfSigned() throws Exception {
CertUtils certUtils = getUtils();
- File tempSubDir = tempDir.toPath().resolve(testName()).toFile();
- assertTrue(tempSubDir.isDirectory() || tempSubDir.mkdir());
- File rootKeyStoreFile = tempSubDir.toPath().resolve("root.jks").toFile();
- certUtils.createSelfSignedCert(rootKeyStoreFile, "test", PASSWORD);
- File publicKeyStoreFile =
tempSubDir.toPath().resolve("public.jks").toFile();
- certUtils.createPublicCert(publicKeyStoreFile, "test",
rootKeyStoreFile.getAbsolutePath(),
+ Path tempSubDir = tempDir.resolve(testName());
+ if (!Files.isDirectory(tempSubDir)) {
+ Files.createDirectories(tempSubDir);
+ }
Review Comment:
could add similar assertion: `assertTrue(Files.isDirectory(dir))` or move to
util method I mentioned earlier
##########
test/src/main/java/org/apache/accumulo/test/ImportExportIT.java:
##########
@@ -94,6 +96,9 @@
*/
public class ImportExportIT extends AccumuloClusterHarness {
+ @TempDir
+ private static java.nio.file.Path tempDir;
+
Review Comment:
I don't think this is used
##########
test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java:
##########
@@ -164,25 +172,27 @@ public void signingChain() throws Exception {
// no reason the keypair we generate for the tservers need to be able to
sign anything,
// but this is a way to make sure the private and public keys created
actually correspond.
CertUtils certUtils = getUtils();
- File tempSubDir = tempDir.toPath().resolve(testName()).toFile();
- assertTrue(tempSubDir.isDirectory() || tempSubDir.mkdir());
- File rootKeyStoreFile = tempSubDir.toPath().resolve("root.jks").toFile();
- certUtils.createSelfSignedCert(rootKeyStoreFile, "test", PASSWORD);
- File signedCaKeyStoreFile =
tempSubDir.toPath().resolve("signedca.jks").toFile();
- certUtils.createSignedCert(signedCaKeyStoreFile, "test", PASSWORD,
- rootKeyStoreFile.getAbsolutePath(), PASSWORD);
- File signedLeafKeyStoreFile =
tempSubDir.toPath().resolve("signedleaf.jks").toFile();
- certUtils.createSignedCert(signedLeafKeyStoreFile, "test", PASSWORD,
- signedCaKeyStoreFile.getAbsolutePath(), PASSWORD);
+ Path tempSubDir = tempDir.resolve(testName());
+ if (!Files.isDirectory(tempSubDir)) {
+ Files.createDirectories(tempSubDir);
+ }
Review Comment:
could add similar assertion: `assertTrue(Files.isDirectory(dir))` or move to
util method I mentioned earlier
##########
test/src/main/java/org/apache/accumulo/test/AuditMessageIT.java:
##########
@@ -124,36 +125,24 @@ private ArrayList<String> getAuditMessages(String
stepName) throws IOException {
// Grab the audit messages
System.out.println("Start of captured audit messages for step " +
stepName);
- ArrayList<String> result = new ArrayList<>();
- File[] files = getCluster().getConfig().getLogDir().listFiles();
- assertNotNull(files);
- for (File file : files) {
- // We want to grab the files called .out
- if (file.getName().contains(".out") && file.isFile() && file.canRead()) {
- try (java.util.Scanner it = new java.util.Scanner(file, UTF_8)) {
- // strip off prefix, because log4j.properties does
- final var pattern = Pattern.compile(".* \\["
- + AuditedSecurityOperation.AUDITLOG.replace("org.apache.",
"").replace(".", "[.]")
- + "\\] .*");
- while (it.hasNext()) {
- String line = it.nextLine();
- if (pattern.matcher(line).matches()) {
- // Only include the message if startTimestamp is null. or the
message occurred after
- // the startTimestamp value
- if ((lastAuditTimestamp == null)
- || (line.substring(0, 23).compareTo(lastAuditTimestamp) >
0)) {
- result.add(line);
- }
+ ArrayList<String> result;
+ final var pattern = Pattern.compile(
+ ".* \\[" + AuditedSecurityOperation.AUDITLOG.replace("org.apache.",
"").replace(".", "[.]")
+ + "\\] .*");
+ try (var paths =
Files.list(getCluster().getConfig().getLogDir().toPath())) {
+ result = paths.filter(file ->
file.getFileName().toString().contains(".out"))
+ .filter(file -> Files.isRegularFile(file) &&
Files.isReadable(file)).flatMap(path -> {
+ try {
+ return Files.lines(path, UTF_8);
+ } catch (IOException e) {
+ throw new RuntimeException(e);
}
- }
- }
- }
+ }).filter(pattern.asPredicate())
+ .filter(line -> lastAuditTimestamp == null
+ || line.substring(0, 23).compareTo(lastAuditTimestamp) > 0)
+
.sorted().peek(System.out::println).collect(Collectors.toCollection(ArrayList::new));
Review Comment:
Should include comments explaining what this is doing, could probably just
include the ones that were there before. Could also condense a couple of the
`filter` calls into one.
##########
test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java:
##########
@@ -126,26 +132,28 @@ public void publicOnlyVerfication() throws Exception {
// this approximates the real life scenario. the client will only have the
public key of each
// cert (the root made by us as below, but the signed cert extracted by
the SSL transport)
CertUtils certUtils = getUtils();
- File tempSubDir = tempDir.toPath().resolve(testName()).toFile();
- assertTrue(tempSubDir.isDirectory() || tempSubDir.mkdir());
- File rootKeyStoreFile = tempSubDir.toPath().resolve("root.jks").toFile();
- certUtils.createSelfSignedCert(rootKeyStoreFile, "test", PASSWORD);
- File publicRootKeyStoreFile =
tempSubDir.toPath().resolve("publicroot.jks").toFile();
- certUtils.createPublicCert(publicRootKeyStoreFile, "test",
rootKeyStoreFile.getAbsolutePath(),
+ Path tempSubDir = tempDir.resolve(testName());
+ if (!Files.isDirectory(tempSubDir)) {
+ Files.createDirectories(tempSubDir);
+ }
Review Comment:
could add similar assertion: `assertTrue(Files.isDirectory(dir))` or move to
util method I mentioned earlier
##########
test/src/main/java/org/apache/accumulo/test/DumpConfigIT.java:
##########
@@ -58,20 +58,22 @@ public void configure(MiniAccumuloConfigImpl cfg,
Configuration hadoopCoreSite)
justification = "user.dir is suitable test input")
@Test
public void test() throws Exception {
- File folder = tempDir.toPath().resolve(testName() + "/").toFile();
- assertTrue(folder.isDirectory() || folder.mkdir(), "failed to create dir:
" + folder);
- File siteFileBackup =
folder.toPath().resolve("accumulo.properties.bak").toFile();
- assertFalse(siteFileBackup.exists());
- assertEquals(0, exec(Admin.class, "dumpConfig", "-a", "-d",
folder.getPath()).waitFor());
- assertTrue(siteFileBackup.exists());
- String site =
FunctionalTestUtils.readAll(Files.newInputStream(siteFileBackup.toPath()));
+ Path folder = tempDir.resolve(testName());
+ if (!Files.isDirectory(folder)) {
+ Files.createDirectories(folder);
+ }
Review Comment:
could add similar assertion: `assertTrue(Files.isDirectory(dir))` or move to
util method I mentioned earlier
##########
test/src/main/java/org/apache/accumulo/test/fate/meta/ZooMutatorIT.java:
##########
@@ -79,9 +80,11 @@ public class ZooMutatorIT extends WithTestNames {
*/
@Test
public void concurrentMutatorTest() throws Exception {
- File newFolder = tempDir.toPath().resolve(testName() + "/").toFile();
- assertTrue(newFolder.isDirectory() || newFolder.mkdir(), "failed to create
dir: " + newFolder);
- try (var testZk = new ZooKeeperTestingServer(newFolder); var zk =
testZk.newClient()) {
+ Path newFolder = tempDir.resolve(testName() + "/");
+ if (!Files.isDirectory(newFolder)) {
+ Files.createDirectories(newFolder);
+ }
Review Comment:
could add similar assertion: `assertTrue(Files.isDirectory(dir))` or move to
util method I mentioned earlier
##########
test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java:
##########
@@ -94,22 +98,24 @@ public void createPublicSelfSigned() throws Exception {
@Test
public void createSigned() throws Exception {
CertUtils certUtils = getUtils();
- File tempSubDir = tempDir.toPath().resolve(testName()).toFile();
- assertTrue(tempSubDir.isDirectory() || tempSubDir.mkdir());
- File rootKeyStoreFile = tempSubDir.toPath().resolve("root.jks").toFile();
- certUtils.createSelfSignedCert(rootKeyStoreFile, "test", PASSWORD);
- File signedKeyStoreFile =
tempSubDir.toPath().resolve("signed.jks").toFile();
- certUtils.createSignedCert(signedKeyStoreFile, "test", PASSWORD,
- rootKeyStoreFile.getAbsolutePath(), PASSWORD);
+ Path tempSubDir = tempDir.resolve(testName());
+ if (!Files.isDirectory(tempSubDir)) {
+ Files.createDirectories(tempSubDir);
+ }
Review Comment:
could add similar assertion: `assertTrue(Files.isDirectory(dir))` or move to
util method I mentioned earlier
##########
test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java:
##########
@@ -53,13 +53,15 @@ private CertUtils getUtils() {
@Test
public void createSelfSigned() throws Exception {
CertUtils certUtils = getUtils();
- File tempSubDir = tempDir.toPath().resolve(testName()).toFile();
- assertTrue(tempSubDir.isDirectory() || tempSubDir.mkdir());
- File keyStoreFile = tempSubDir.toPath().resolve("selfsigned.jks").toFile();
- certUtils.createSelfSignedCert(keyStoreFile, "test", PASSWORD);
+ Path tempSubDir = tempDir.resolve(testName());
+ if (!Files.isDirectory(tempSubDir)) {
+ Files.createDirectories(tempSubDir);
+ }
Review Comment:
could add similar assertion: `assertTrue(Files.isDirectory(dir))` or move to
util method I mentioned earlier
--
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]
