Committed by
Gerrit Code Review
Intent manager unit test enhancements
- add test to be sure that all flows are removed when intents are withdrawn - add test to be sure that all flows are removed when an intent installation fails. Currently disabled, intent cleanup is not implemented - enable installation time out test Change-Id: I8c7a75292a97404ef89856647c67ef2f70ffcf6f
Showing
4 changed files
with
127 additions
and
24 deletions
| ... | @@ -330,18 +330,23 @@ public class IntentTestsMocks { | ... | @@ -330,18 +330,23 @@ public class IntentTestsMocks { |
| 330 | new IntentTestsMocks.MockTreatment(); | 330 | new IntentTestsMocks.MockTreatment(); |
| 331 | 331 | ||
| 332 | public static class MockFlowRule implements FlowRule { | 332 | public static class MockFlowRule implements FlowRule { |
| 333 | + static int nextId = 0; | ||
| 333 | 334 | ||
| 334 | int priority; | 335 | int priority; |
| 335 | Type type; | 336 | Type type; |
| 337 | + long timestamp; | ||
| 338 | + int id; | ||
| 336 | 339 | ||
| 337 | public MockFlowRule(int priority) { | 340 | public MockFlowRule(int priority) { |
| 338 | this.priority = priority; | 341 | this.priority = priority; |
| 339 | this.type = Type.DEFAULT; | 342 | this.type = Type.DEFAULT; |
| 343 | + this.timestamp = System.currentTimeMillis(); | ||
| 344 | + this.id = nextId++; | ||
| 340 | } | 345 | } |
| 341 | 346 | ||
| 342 | @Override | 347 | @Override |
| 343 | public FlowId id() { | 348 | public FlowId id() { |
| 344 | - return FlowId.valueOf(1); | 349 | + return FlowId.valueOf(id); |
| 345 | } | 350 | } |
| 346 | 351 | ||
| 347 | @Override | 352 | @Override |
| ... | @@ -398,7 +403,8 @@ public class IntentTestsMocks { | ... | @@ -398,7 +403,8 @@ public class IntentTestsMocks { |
| 398 | return false; | 403 | return false; |
| 399 | } | 404 | } |
| 400 | final MockFlowRule other = (MockFlowRule) obj; | 405 | final MockFlowRule other = (MockFlowRule) obj; |
| 401 | - return Objects.equals(this.priority, other.priority); | 406 | + return Objects.equals(this.timestamp, other.timestamp) && |
| 407 | + this.id == other.id; | ||
| 402 | } | 408 | } |
| 403 | 409 | ||
| 404 | @Override | 410 | @Override | ... | ... |
| ... | @@ -23,6 +23,8 @@ import java.util.Map; | ... | @@ -23,6 +23,8 @@ import java.util.Map; |
| 23 | import java.util.Set; | 23 | import java.util.Set; |
| 24 | import java.util.concurrent.CountDownLatch; | 24 | import java.util.concurrent.CountDownLatch; |
| 25 | import java.util.concurrent.TimeUnit; | 25 | import java.util.concurrent.TimeUnit; |
| 26 | +import java.util.stream.Collectors; | ||
| 27 | +import java.util.stream.IntStream; | ||
| 26 | 28 | ||
| 27 | import org.hamcrest.Description; | 29 | import org.hamcrest.Description; |
| 28 | import org.hamcrest.TypeSafeMatcher; | 30 | import org.hamcrest.TypeSafeMatcher; |
| ... | @@ -57,6 +59,7 @@ import com.google.common.collect.Sets; | ... | @@ -57,6 +59,7 @@ import com.google.common.collect.Sets; |
| 57 | 59 | ||
| 58 | import static org.hamcrest.MatcherAssert.assertThat; | 60 | import static org.hamcrest.MatcherAssert.assertThat; |
| 59 | import static org.hamcrest.Matchers.hasSize; | 61 | import static org.hamcrest.Matchers.hasSize; |
| 62 | +import static org.hamcrest.Matchers.is; | ||
| 60 | import static org.junit.Assert.assertEquals; | 63 | import static org.junit.Assert.assertEquals; |
| 61 | import static org.junit.Assert.assertNotNull; | 64 | import static org.junit.Assert.assertNotNull; |
| 62 | import static org.junit.Assert.assertTrue; | 65 | import static org.junit.Assert.assertTrue; |
| ... | @@ -164,6 +167,18 @@ public class IntentManagerTest { | ... | @@ -164,6 +167,18 @@ public class IntentManagerTest { |
| 164 | } | 167 | } |
| 165 | } | 168 | } |
| 166 | 169 | ||
| 170 | + private static class TestIntentCompilerMultipleFlows implements IntentCompiler<MockIntent> { | ||
| 171 | + @Override | ||
| 172 | + public List<Intent> compile(MockIntent intent, List<Intent> installable, | ||
| 173 | + Set<LinkResourceAllocations> resources) { | ||
| 174 | + | ||
| 175 | + return IntStream.rangeClosed(1, 5) | ||
| 176 | + .mapToObj(mock -> (new MockInstallableIntent())) | ||
| 177 | + .collect(Collectors.toList()); | ||
| 178 | + } | ||
| 179 | + } | ||
| 180 | + | ||
| 181 | + | ||
| 167 | private static class TestIntentCompilerError implements IntentCompiler<MockIntent> { | 182 | private static class TestIntentCompilerError implements IntentCompiler<MockIntent> { |
| 168 | @Override | 183 | @Override |
| 169 | public List<Intent> compile(MockIntent intent, List<Intent> installable, | 184 | public List<Intent> compile(MockIntent intent, List<Intent> installable, |
| ... | @@ -173,7 +188,7 @@ public class IntentManagerTest { | ... | @@ -173,7 +188,7 @@ public class IntentManagerTest { |
| 173 | } | 188 | } |
| 174 | 189 | ||
| 175 | /** | 190 | /** |
| 176 | - * Hamcrest matcher to check that a conllection of Intents contains an | 191 | + * Hamcrest matcher to check that a collection of Intents contains an |
| 177 | * Intent with the specified Intent Id. | 192 | * Intent with the specified Intent Id. |
| 178 | */ | 193 | */ |
| 179 | public static class EntryForIntentMatcher extends TypeSafeMatcher<Collection<Intent>> { | 194 | public static class EntryForIntentMatcher extends TypeSafeMatcher<Collection<Intent>> { |
| ... | @@ -375,7 +390,6 @@ public class IntentManagerTest { | ... | @@ -375,7 +390,6 @@ public class IntentManagerTest { |
| 375 | * Tests handling a future that contains an unresolvable error as a result of | 390 | * Tests handling a future that contains an unresolvable error as a result of |
| 376 | * installing an intent. | 391 | * installing an intent. |
| 377 | */ | 392 | */ |
| 378 | - @Ignore("test needs to be rewritten, so that the intent is resubmitted") | ||
| 379 | @Test | 393 | @Test |
| 380 | public void errorIntentInstallNeverTrue() { | 394 | public void errorIntentInstallNeverTrue() { |
| 381 | final Long id = MockIntent.nextId(); | 395 | final Long id = MockIntent.nextId(); |
| ... | @@ -489,4 +503,78 @@ public class IntentManagerTest { | ... | @@ -489,4 +503,78 @@ public class IntentManagerTest { |
| 489 | assertThat(intents, hasIntentWithId(intent2.id())); | 503 | assertThat(intents, hasIntentWithId(intent2.id())); |
| 490 | verifyState(); | 504 | verifyState(); |
| 491 | } | 505 | } |
| 506 | + | ||
| 507 | + /** | ||
| 508 | + * Tests that removing all intents results in no flows remaining. | ||
| 509 | + */ | ||
| 510 | + @Test | ||
| 511 | + public void testFlowRemoval() { | ||
| 512 | + List<Intent> intents; | ||
| 513 | + | ||
| 514 | + flowRuleService.setFuture(true); | ||
| 515 | + | ||
| 516 | + intents = Lists.newArrayList(service.getIntents()); | ||
| 517 | + assertThat(intents, hasSize(0)); | ||
| 518 | + | ||
| 519 | + final MockIntent intent1 = new MockIntent(MockIntent.nextId()); | ||
| 520 | + final MockIntent intent2 = new MockIntent(MockIntent.nextId()); | ||
| 521 | + | ||
| 522 | + listener.setLatch(1, Type.INSTALL_REQ); | ||
| 523 | + listener.setLatch(1, Type.INSTALLED); | ||
| 524 | + | ||
| 525 | + service.submit(intent1); | ||
| 526 | + listener.await(Type.INSTALL_REQ); | ||
| 527 | + listener.await(Type.INSTALLED); | ||
| 528 | + | ||
| 529 | + | ||
| 530 | + listener.setLatch(1, Type.INSTALL_REQ); | ||
| 531 | + listener.setLatch(1, Type.INSTALLED); | ||
| 532 | + | ||
| 533 | + service.submit(intent2); | ||
| 534 | + listener.await(Type.INSTALL_REQ); | ||
| 535 | + listener.await(Type.INSTALLED); | ||
| 536 | + | ||
| 537 | + assertThat(listener.getCounts(Type.INSTALLED), is(2)); | ||
| 538 | + assertThat(flowRuleService.getFlowRuleCount(), is(2)); | ||
| 539 | + | ||
| 540 | + listener.setLatch(1, Type.WITHDRAWN); | ||
| 541 | + service.withdraw(intent1); | ||
| 542 | + listener.await(Type.WITHDRAWN); | ||
| 543 | + | ||
| 544 | + listener.setLatch(1, Type.WITHDRAWN); | ||
| 545 | + service.withdraw(intent2); | ||
| 546 | + listener.await(Type.WITHDRAWN); | ||
| 547 | + | ||
| 548 | + assertThat(listener.getCounts(Type.WITHDRAWN), is(2)); | ||
| 549 | + assertThat(flowRuleService.getFlowRuleCount(), is(0)); | ||
| 550 | + } | ||
| 551 | + | ||
| 552 | + /** | ||
| 553 | + * Tests that an intent that fails installation results in no flows remaining. | ||
| 554 | + */ | ||
| 555 | + @Test | ||
| 556 | + @Ignore("Cleanup state is not yet implemented in the intent manager") | ||
| 557 | + public void testFlowRemovalInstallError() { | ||
| 558 | + final TestIntentCompilerMultipleFlows errorCompiler = new TestIntentCompilerMultipleFlows(); | ||
| 559 | + extensionService.registerCompiler(MockIntent.class, errorCompiler); | ||
| 560 | + List<Intent> intents; | ||
| 561 | + | ||
| 562 | + flowRuleService.setFuture(true); | ||
| 563 | + flowRuleService.setErrorFlow(3); | ||
| 564 | + | ||
| 565 | + intents = Lists.newArrayList(service.getIntents()); | ||
| 566 | + assertThat(intents, hasSize(0)); | ||
| 567 | + | ||
| 568 | + final MockIntent intent1 = new MockIntent(MockIntent.nextId()); | ||
| 569 | + | ||
| 570 | + listener.setLatch(1, Type.INSTALL_REQ); | ||
| 571 | + listener.setLatch(1, Type.FAILED); | ||
| 572 | + | ||
| 573 | + service.submit(intent1); | ||
| 574 | + listener.await(Type.INSTALL_REQ); | ||
| 575 | + listener.await(Type.FAILED); | ||
| 576 | + | ||
| 577 | + assertThat(listener.getCounts(Type.FAILED), is(1)); | ||
| 578 | + assertThat(flowRuleService.getFlowRuleCount(), is(0)); | ||
| 579 | + } | ||
| 492 | } | 580 | } | ... | ... |
| ... | @@ -34,6 +34,11 @@ public class MockFlowRuleService extends FlowRuleServiceAdapter { | ... | @@ -34,6 +34,11 @@ public class MockFlowRuleService extends FlowRuleServiceAdapter { |
| 34 | final Set<FlowRule> flows = Sets.newHashSet(); | 34 | final Set<FlowRule> flows = Sets.newHashSet(); |
| 35 | boolean success; | 35 | boolean success; |
| 36 | 36 | ||
| 37 | + int errorFlow = -1; | ||
| 38 | + public void setErrorFlow(int errorFlow) { | ||
| 39 | + this.errorFlow = errorFlow; | ||
| 40 | + } | ||
| 41 | + | ||
| 37 | public void setFuture(boolean success) { | 42 | public void setFuture(boolean success) { |
| 38 | this.success = success; | 43 | this.success = success; |
| 39 | } | 44 | } |
| ... | @@ -41,16 +46,20 @@ public class MockFlowRuleService extends FlowRuleServiceAdapter { | ... | @@ -41,16 +46,20 @@ public class MockFlowRuleService extends FlowRuleServiceAdapter { |
| 41 | @Override | 46 | @Override |
| 42 | public void apply(FlowRuleOperations ops) { | 47 | public void apply(FlowRuleOperations ops) { |
| 43 | ops.stages().forEach(stage -> stage.forEach(flow -> { | 48 | ops.stages().forEach(stage -> stage.forEach(flow -> { |
| 44 | - switch (flow.type()) { | 49 | + if (errorFlow == flow.rule().id().value()) { |
| 45 | - case ADD: | 50 | + success = false; |
| 46 | - case MODIFY: //TODO is this the right behavior for modify? | 51 | + } else { |
| 47 | - flows.add(flow.rule()); | 52 | + switch (flow.type()) { |
| 48 | - break; | 53 | + case ADD: |
| 49 | - case REMOVE: | 54 | + case MODIFY: //TODO is this the right behavior for modify? |
| 50 | - flows.remove(flow.rule()); | 55 | + flows.add(flow.rule()); |
| 51 | - break; | 56 | + break; |
| 52 | - default: | 57 | + case REMOVE: |
| 53 | - break; | 58 | + flows.remove(flow.rule()); |
| 59 | + break; | ||
| 60 | + default: | ||
| 61 | + break; | ||
| 62 | + } | ||
| 54 | } | 63 | } |
| 55 | })); | 64 | })); |
| 56 | if (success) { | 65 | if (success) { | ... | ... |
| ... | @@ -99,19 +99,19 @@ public class SimpleIntentStore | ... | @@ -99,19 +99,19 @@ public class SimpleIntentStore |
| 99 | IntentData pendingData = pending.get(newData.key()); | 99 | IntentData pendingData = pending.get(newData.key()); |
| 100 | 100 | ||
| 101 | if (IntentData.isUpdateAcceptable(currentData, newData)) { | 101 | if (IntentData.isUpdateAcceptable(currentData, newData)) { |
| 102 | - if (pendingData.state() == PURGE_REQ) { | 102 | + if (pendingData != null) { |
| 103 | - current.remove(newData.key(), newData); | 103 | + if (pendingData.state() == PURGE_REQ) { |
| 104 | - } else { | 104 | + current.remove(newData.key(), newData); |
| 105 | - current.put(newData.key(), new IntentData(newData)); | 105 | + } else { |
| 106 | - } | 106 | + current.put(newData.key(), new IntentData(newData)); |
| 107 | - | 107 | + } |
| 108 | - if (pendingData != null | 108 | + |
| 109 | + if (pendingData.version().compareTo(newData.version()) <= 0) { | ||
| 109 | // pendingData version is less than or equal to newData's | 110 | // pendingData version is less than or equal to newData's |
| 110 | // Note: a new update for this key could be pending (it's version will be greater) | 111 | // Note: a new update for this key could be pending (it's version will be greater) |
| 111 | - && pendingData.version().compareTo(newData.version()) <= 0) { | 112 | + pending.remove(newData.key()); |
| 112 | - pending.remove(newData.key()); | 113 | + } |
| 113 | } | 114 | } |
| 114 | - | ||
| 115 | notifyDelegateIfNotNull(IntentEvent.getEvent(newData)); | 115 | notifyDelegateIfNotNull(IntentEvent.getEvent(newData)); |
| 116 | } | 116 | } |
| 117 | } | 117 | } | ... | ... |
-
Please register or login to post a comment