sarutak commented on code in PR #57676:
URL: https://github.com/apache/spark/pull/57676#discussion_r3691154218
##########
core/src/test/scala/org/apache/spark/deploy/history/EventLogFileWritersSuite.scala:
##########
@@ -352,6 +352,17 @@ class SingleEventLogFileWriterSuite extends
EventLogFileWritersSuite {
"a fine:mind$dollar{bills}.1", None, Some(CompressionCodec.LZ4)))
}
+ test("Event log file names") {
Review Comment:
Could we add a test case for `getLogFileNames("app1", None)` as well? It
would cover the path
where no attempt ID suffix is appended.
##########
core/src/test/scala/org/apache/spark/deploy/history/FsHistoryProviderSuite.scala:
##########
@@ -1759,6 +1761,151 @@ abstract class FsHistoryProviderSuite extends
SparkFunSuite with Matchers with P
}
}
+ test("Support spark.history.fs.eventLog.single.onDemandLoadEnabled") {
Review Comment:
The tests properly isolate single vs rolling by disabling the other. Would
it be worth adding one test with both enabled (the default state) to verify the
combined fallback behavior?
Also, the existing rolling on-demand test checks
`provider.getAppUI("nonexist", None).isEmpty` to ensure no phantom entries are
left. Could we add the same assertion to the single on-demand test?
--
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]
