HeartSaVioR commented on code in PR #37474:
URL: https://github.com/apache/spark/pull/37474#discussion_r950767972
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/streaming/CheckpointFileManagerSuite.scala:
##########
@@ -58,50 +77,40 @@ abstract class CheckpointFileManagerTests extends
SparkFunSuite with SQLHelper {
// Create atomic without overwrite
var path = new Path(s"$dir/file")
assert(!fm.exists(path))
- fm.createAtomic(path, overwriteIfPossible = false).cancel()
+ fm.createAtomic(path, overwriteIfPossible =
false).writeContent(1).cancel()
assert(!fm.exists(path))
- fm.createAtomic(path, overwriteIfPossible = false).close()
+ fm.createAtomic(path, overwriteIfPossible =
false).writeContent(2).close()
assert(fm.exists(path))
+ assert(fm.open(path).readContent() == 2)
quietly {
intercept[IOException] {
// should throw exception since file exists and overwrite is false
- fm.createAtomic(path, overwriteIfPossible = false).close()
+ fm.createAtomic(path, overwriteIfPossible =
false).writeContent(3).close()
}
}
+ assert(fm.open(path).readContent() == 2)
// Create atomic with overwrite if possible
path = new Path(s"$dir/file2")
assert(!fm.exists(path))
- fm.createAtomic(path, overwriteIfPossible = true).cancel()
+ fm.createAtomic(path, overwriteIfPossible =
true).writeContent(4).cancel()
assert(!fm.exists(path))
- fm.createAtomic(path, overwriteIfPossible = true).close()
+ fm.createAtomic(path, overwriteIfPossible = true).writeContent(5).close()
assert(fm.exists(path))
- fm.createAtomic(path, overwriteIfPossible = true).close() // should not
throw exception
-
- // crc file should not be leaked when origin file doesn't exist.
- // The implementation of Hadoop filesystem may filter out checksum file,
so
- // listing files from local filesystem.
- val fileNames = new File(path.getParent.toString).listFiles().toSeq
- .filter(p => p.isFile).map(p => p.getName)
- val crcFiles = fileNames.filter(n => n.startsWith(".") &&
n.endsWith(".crc"))
- val originFileNamesForExistingCrcFiles = crcFiles.map { name =>
- // remove first "." and last ".crc"
- name.substring(1, name.length - 4)
- }
-
- // Check all origin files exist for all crc files.
-
assert(originFileNamesForExistingCrcFiles.toSet.subsetOf(fileNames.toSet),
- s"Some of origin files for crc files don't exist - crc files:
$crcFiles / " +
- s"expected origin files: $originFileNamesForExistingCrcFiles /
actual files: $fileNames")
+ assert(fm.open(path).readContent() == 5)
+ // should not throw exception
+ fm.createAtomic(path, overwriteIfPossible = true).writeContent(6).close()
+ assert(fm.open(path).readContent() == 6)
+ checkLeakingCrcFiles(dir)
// Open and delete
fm.open(path).close()
fm.delete(path)
assert(!fm.exists(path))
intercept[IOException] {
fm.open(path)
}
- fm.delete(path) // should not throw exception
+ fm.delete(dir) // should not throw exception
Review Comment:
Yeah, actually this intends to "test" the behavior (triggering delete
against non-existing file does not throw exception), not to clean up the
resource.
--
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]
