Github user His-name-is-Joof commented on a diff in the pull request:

    https://github.com/apache/spark/pull/6380#discussion_r34835331
  
    --- Diff: 
streaming/src/test/scala/org/apache/spark/streaming/StreamingListenerSuite.scala
 ---
    @@ -140,6 +141,16 @@ class StreamingListenerSuite extends TestSuiteBase 
with Matchers {
         }
         true
       }
    +
    +  test("registering listeners via spark.streaming.extraListeners") {
    +    val conf = new SparkConf().setMaster("local").setAppName("test")
    +      .set("spark.streaming.extraListeners", 
classOf[BatchInfoCollector].getName + "," +
    --- End diff --
    
    The change is already in StreamingListenerSuite.scala (and the class
    StreamingListenerSuite) which seems appropriate. Did you mean create new
    classes within StreamingListenerSuite.scala to handle the test cases?
    
    On Mon, Jul 13, 2015 at 5:36 PM, Tathagata Das <[email protected]>
    wrote:
    
    > In
    > 
streaming/src/test/scala/org/apache/spark/streaming/StreamingListenerSuite.scala
    > <https://github.com/apache/spark/pull/6380#discussion_r34524259>:
    >
    > > @@ -140,6 +141,16 @@ class StreamingListenerSuite extends TestSuiteBase 
with Matchers {
    > >      }
    > >      true
    > >    }
    > > +
    > > +  test("registering listeners via spark.streaming.extraListeners") {
    > > +    val conf = new SparkConf().setMaster("local").setAppName("test")
    > > +      .set("spark.streaming.extraListeners", 
classOf[BatchInfoCollector].getName + "," +
    >
    > I missed it earlier, this unit does not cover the two cases, one without
    > constructor and one with constructor.
    > It may be easier to read if you create new classes -
    > StreamingListenerWithoutArg, and StreamingListenerWithConf
    > (similar to ListenerThatAcceptsSparkConf for spark.extraListeners in
    > SparkListenerSuite)
    >
    > Also, the StreamingContextSuite is getting really overloaded, mind
    > shifting this to StreamingListenerSuite, and reverting this file?
    >
    > —
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/spark/pull/6380/files#r34524259>.
    >



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to