Skip to content

Conversation

@revans2
Copy link
Contributor

@revans2 revans2 commented Sep 20, 2017

No description provided.

@revans2 revans2 force-pushed the STORM-2748 branch 2 times, most recently from 4701ee3 to a7304c3 Compare September 20, 2017 16:14
Copy link
Contributor

@srdo srdo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 pending tests passing, thanks for fixing this.

public void prepare(Map<String, Object> conf, TopologyContext topologyContext, OutputCollector outputCollector) {}
@Override
public void execute(Tuple tuple) {}
private static class NoopBlot extends BaseRichBolt {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Blot -> Bolt

public void cleanup() { }
@Override
public void execute(Tuple tuple) {
LOG.error("GOT {}", tuple);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Info or debug might be nicer so a grep for errors in the logs doesn't produce this line.

@revans2
Copy link
Contributor Author

revans2 commented Sep 20, 2017

@srdo thanks for the review. Trying to get the test to work on the slower VMs is still a work in progress. I'll let you know why I actually have it passing consistently.

@revans2
Copy link
Contributor Author

revans2 commented Sep 20, 2017

@srdo I finally have the test passing predictably on travis. I also fixed the issues that you saw. Feel free to take another look.

Copy link
Contributor

@srdo srdo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 again

}
tickTupleTimes.clear();
cluster.advanceClusterTime(1);
time += 1000;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be put in a loop now, the following lines don't differ anymore.

@revans2
Copy link
Contributor Author

revans2 commented Sep 20, 2017

@srdo I turned it into a loop like you requested

@srdo
Copy link
Contributor

srdo commented Sep 21, 2017

Thanks

@HeartSaVioR
Copy link
Contributor

+1

@asfgit asfgit merged commit b182dc6 into apache:master Sep 21, 2017
asfgit pushed a commit that referenced this pull request Sep 21, 2017
… into STORM-2748

STORM-2748: Fix TickTupleTest to actually test something

This closes #2334
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants