Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use EventCollector if possible. #936

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

stefanbirkner
Copy link
Contributor

Avoids duplicate implementations of RunListener for different test and
provides Hamcrest matchers for verifying events sent to the listener.

@stefanbirkner stefanbirkner changed the title Use EventCollecter if possible. Use EventCollector if possible. Jun 23, 2014
*/
public class EventCollector extends RunListener {

private final List<Description> fTestRunsStarted = synchronizedList(new ArrayList<Description>());
Copy link
Member

Choose a reason for hiding this comment

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

We no longer use an f prefix for fields

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code is much older :-)

Done.

@kcooney
Copy link
Member

kcooney commented Jun 28, 2014

Very nice. I do think some of the test support code could use some tests. Don't go crazy; "test until fear turns to boredom" :-)

@kcooney
Copy link
Member

kcooney commented Jun 29, 2014

Were you going to add tests for some of the non-trivial code in the testsupport package?

@stefanbirkner
Copy link
Contributor Author

Yes. I already started.

@marcphilipp
Copy link
Member

@stefanbirkner LGTM! Do you want to take another look before we merge it?

@marcphilipp
Copy link
Member

Needs squashing, though.

Avoids duplicate implementations of RunListener for different test and
provides Hamcrest matchers for verifying events sent to the listener.
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.

3 participants