Refactoring AbstractRib and adding tests #598
Conversation
|
|
||
| /* | ||
| If prefixes don't match exactly, look at the current bit. That determines whether we look | ||
| left of right. As long as the child is not null, recurse. |
There was a problem hiding this comment.
Either use Java multiline comment form
/*
* ...
*/or Java single-line block comment form:
// Line 1
// Line 2
// Line 3|
|
||
| // Assertions | ||
| // Check routes size | ||
| assertThat(_rib.getRoutes().size(), is(1)); |
There was a problem hiding this comment.
can use Matchers.hasSize for this, I think. Assuming it's a Collection, not a Map.
|
|
||
| @Test | ||
| public void testRibConstructor() { | ||
| // Test: the setupEmptyRib() fixture |
There was a problem hiding this comment.
Drop these comments -- structuring the test in a logical order with, optionally, whitespace between sections is good enough :)
|
|
||
| @Test | ||
| public void testMultiOverlappingRouteAdd() { | ||
| // Setup/Test: Add multiple routes with overlapping prefixes |
There was a problem hiding this comment.
This type of comment where useful for understanding test goals should go in the function javadoc.
|
|
||
| match = _rib.longestPrefixMatch(new Ip("10.1.1.2")); | ||
| assertThat(match.size(), equalTo(1)); | ||
| assertThat(match, contains(routes.get(1))); |
There was a problem hiding this comment.
also test one that doesn't match (even though the rib is not empty ;).
| import org.junit.Test; | ||
|
|
||
| /** | ||
| * Test the AbstractRib tree logic. To avoid worrying about route preference comparisons, testing |
There was a problem hiding this comment.
Simpler:
/** Tests of {@link AbstractRib}. */Then move the bit about StaticRoute to the _rib variable.
My general rule here is that public Javadoc is for people looking at "What does X do?" whereas private documentation is for people reading the code ("how does X work?").
[And, I don't think I care how we test AbstractRib until I'm reading the code.]
| */ | ||
| public class AbstractRibTest { | ||
| private AbstractRib<StaticRoute> _rib; | ||
| private StaticRoute _mostGeneralRoute; |
There was a problem hiding this comment.
private static final StaticRoute MOST_GENERAL_ROUTE = ...
Since it's a constant that will never change per test.
Fixes #591