Added Memory Appender, Log assertions in OpSparkListner #62
Conversation
ajayborra
commented
Aug 16, 2018
- Added Memory Appender, Log assertions for OpSparkListner
Codecov Report
@@ Coverage Diff @@
## master #62 +/- ##
==========================================
- Coverage 85.79% 85.79% -0.01%
==========================================
Files 298 299 +1
Lines 9760 9771 +11
Branches 326 533 +207
==========================================
+ Hits 8374 8383 +9
- Misses 1386 1388 +2
Continue to review full report at Codecov.
|
| import scala.collection.mutable | ||
|
|
||
| class MemoryAppender extends AppenderSkeleton { | ||
| private val _logs = new mutable.HashSet[spi.LoggingEvent] |
There was a problem hiding this comment.
- no need to prefix with
_ - make this an
ArrayBuffer
There was a problem hiding this comment.
Was using hash sets for constant inset and lookup times. Array buf's seems to be a good choice as well, will change it.
There was a problem hiding this comment.
Hashset would dedup and loose order, thereforce we cannot use it.
| @@ -0,0 +1,74 @@ | |||
| /* | |||
There was a problem hiding this comment.
define this appender in the same file as OpSparkListenerTest - we dont need to expose it as it's currently only useful in one test.
| @@ -44,6 +44,15 @@ trait TestSparkContext extends TempDirectoryTest with TestCommon { | |||
| classOf[com.salesforce.op.test.PassengerCSV] | |||
| ) | |||
|
|
|||
| val sparkLogAppender: MemoryAppender = { | |||
There was a problem hiding this comment.
we dont need this in every test. let's only use it in OpSparkListenerTest
| @@ -12,3 +12,4 @@ log4j.logger.org.eclipse.jetty=ERROR | |||
|
|
|||
| # TransmogrifAI logging | |||
| log4j.logger.com.salesforce.op=ERROR | |||
| log4j.logger.com.salesforce.op.utils.spark.OpSparkListener=INFO | |||
There was a problem hiding this comment.
you can set this log4j.logger.com.salesforce.op.utils.spark.OpSparkListener=INFO programmatically in OpSparkListenerTest
| * @param logMessage The log message | ||
| * @return Boolean of log existence | ||
| */ | ||
| def logExists(logLevel: Level, logMessage: String): Boolean = { |
There was a problem hiding this comment.
this seems a bit redundant. you can simply do this check in the test itself.
|
Closing this PR in favor of #69. As the link b/w this PR and the branch on old fork broke, post making repo public. |