Github user srdo commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2443#discussion_r155520270
  
    --- Diff: 
sql/storm-sql-runtime/src/test/org/apache/storm/sql/TestUtils.java ---
    @@ -190,363 +191,213 @@ public void open(ChannelContext ctx) {
         }
       }
     
    -  public static class MockState implements State {
    -    /**
    -     * Collect all values in a static variable as the instance will go 
through serialization and deserialization.
    -     * NOTE: This should be cleared before or after running each test.
    -     */
    -    private transient static final List<List<Object> > VALUES = new 
ArrayList<>();
    +  public static class MockSpout extends BaseRichSpout {
     
    -    public static List<List<Object>> getCollectedValues() {
    -      return VALUES;
    +    private final List<Values> records;
    +    private final Fields outputFields;
    +    private boolean emitted = false;
    +    private SpoutOutputCollector collector;
    +
    +    public MockSpout(List<Values> records, Fields outputFields) {
    +      this.records = records;
    +      this.outputFields = outputFields;
         }
     
         @Override
    -    public void beginCommit(Long txid) {
    -      // NOOP
    +    public void open(Map<String, Object> conf, TopologyContext context, 
SpoutOutputCollector collector) {
    +      this.collector = collector;
         }
     
         @Override
    -    public void commit(Long txid) {
    -      // NOOP
    -    }
    +    public void nextTuple() {
    +      if (emitted) {
    +        return;
    +      }
     
    -    public void updateState(List<TridentTuple> tuples, TridentCollector 
collector) {
    -      for (TridentTuple tuple : tuples) {
    -        VALUES.add(tuple.getValues());
    +      for (Values r : records) {
    +        collector.emit(r);
           }
    -    }
    -  }
     
    -  public static class MockStateFactory implements StateFactory {
    +      emitted = true;
    +    }
     
         @Override
    -    public State makeState(Map<String, Object> conf, IMetricsContext 
metrics, int partitionIndex, int numPartitions) {
    -      return new MockState();
    +    public void declareOutputFields(OutputFieldsDeclarer declarer) {
    +      declarer.declare(outputFields);
         }
       }
     
    -  public static class MockStateUpdater implements StateUpdater<MockState> {
    +  public static class MockBolt extends BaseRichBolt {
    --- End diff --
    
    I think it could be helpful to use a JUnit TestRule (e.g. by subclassing 
http://junit.org/junit4/javadoc/4.12/org/junit/rules/ExternalResource.html) to 
help remember to clear VALUES after the test. It's easy to forget to clean up, 
and then the tests leak state.
    
    Same for the other test utils that keep static state.


---

Reply via email to