llama90 commented on code in PR #42115:
URL: https://github.com/apache/arrow/pull/42115#discussion_r1636427500
##########
java/tools/src/main/java/org/apache/arrow/tools/FileRoundtrip.java:
##########
@@ -52,13 +52,29 @@ public static void main(String[] args) {
System.exit(new FileRoundtrip(System.err).run(args));
}
- private File validateFile(String type, String fileName) {
+ private File validateFile(String type, String fileName) throws IOException {
if (fileName == null) {
throw new IllegalArgumentException("missing " + type + " file
parameter");
}
File f = new File(fileName);
- if (!f.exists() || f.isDirectory()) {
- throw new IllegalArgumentException(type + " file not found: " +
f.getAbsolutePath());
+ if (type.equals("input")) {
+ if (!f.exists() || f.isDirectory()) {
+ throw new IllegalArgumentException(type + " file not found: " +
f.getAbsolutePath());
+ }
+ } else if (type.equals("output")) {
+ File parentDir = f.getParentFile();
+ if (parentDir != null && !parentDir.exists()) {
+ if (!parentDir.mkdirs()) {
+ throw new IOException(
+ "Failed to create parent directory: " +
parentDir.getAbsolutePath());
+ }
+ }
+ if (!f.exists()) {
Review Comment:
In JUnit 4, there was an annotation called `@TemporaryFolder`, but in JUnit
5, it's changed to `@TempDir`. This means that the way temporary files are
created has changed too.
So, just changing the annotation in the old test code wasn't enough to make
the tests pass. I had to add some code to create the `output` file using
`createNewFile()` to make the tests work.
https://github.com/apache/arrow/blob/298786a2e60fdc531b0438a14a641dfc707a8124/java/tools/src/test/java/org/apache/arrow/tools/TestFileRoundtrip.java#L55-L59
While working with the `FileRoundtrip` class, I thought of two ways to
handle this (you can check the link for details). The `input` is always needed,
but the `output` path or file might not always exist (especially if it's
different from the `input` path).
So, I added some code to create the `output` path or file if they don't
exist, before the rest of the logic runs.
If you think there are better ways to do this or if something's unclear, I'd
appreciate your advice.
Here’s the link explaining why this change was needed.
- https://github.com/apache/arrow/pull/42093#discussion_r1634219170
--
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]
