Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-43767][SQL][TESTS] Fix bug in AvroSuite for 'reading from invalid path throws exception' #41289

Closed
wants to merge 2 commits into from

Conversation

panbingkun
Copy link
Contributor

What changes were proposed in this pull request?

The pr aims to fix bug in AvroSuite for 'reading from invalid path throws exception'.

Why are the changes needed?

  • As discussed and analyzed in 41271

  • There is a problem with this UT. Its original intention was to test if there is no file with .avro extensions in the directory, and the read should fail. However, this UT triggered the error as FileUtils.touch instead of spark.read.format("avro").load(dir.toString).The root cause for the failure of this case is that the parent directory was not created. When FileUtils.touch is called in version 1.11.0, it just throws java.io.FileNotFoundException, which covers the error.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass GA.

@panbingkun
Copy link
Contributor Author

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for making an independent PR. It seems that you want to use withTempDir instead of withTempPath + mkdirs. Did I understand correctly?

The root cause for the failure of this case is that the parent directory was not created.

protected def withTempDir(f: File => Unit): Unit = {
val dir = Utils.createTempDir()
try f(dir) finally {
Utils.deleteRecursively(dir)
}
}

@panbingkun
Copy link
Contributor Author

panbingkun commented May 24, 2023

withTempDir

Yep, Perhaps using withTempDir code here is more concise,
Let me do it?
Updated.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM. Thank you for updates.

@panbingkun panbingkun deleted the SPARK-43767 branch June 3, 2023 13:04
czxm pushed a commit to czxm/spark that referenced this pull request Jun 12, 2023
…lid path throws exception'

### What changes were proposed in this pull request?
The pr aims to fix bug in AvroSuite for 'reading from invalid path throws exception'.

### Why are the changes needed?
- As discussed and analyzed in [41271](apache#41271 (comment))

- There is a problem with this UT. Its original intention was to test if there is no file with .avro extensions in the directory, and the read should fail. However, this UT triggered the error as FileUtils.touch instead of spark.read.format("avro").load(dir.toString).The root cause for the failure of this case is that the parent directory was not created. When FileUtils.touch is called in version 1.11.0, it just throws java.io.FileNotFoundException, which covers the error.

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
Pass GA.

Closes apache#41289 from panbingkun/SPARK-43767.

Authored-by: panbingkun <pbk1982@gmail.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants