You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2017/10/28 10:03:55 UTC
[2/3] brooklyn-server git commit: fix broken sensor order test,
clarify expected behaviour
fix broken sensor order test, clarify expected behaviour
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/16a44c25
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/16a44c25
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/16a44c25
Branch: refs/heads/master
Commit: 16a44c25a0908dc4ce68bdcdd3e108b6c1fba259
Parents: bc0b392
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Mon Oct 23 09:25:37 2017 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Mon Oct 23 09:25:37 2017 +0100
----------------------------------------------------------------------
.../apache/brooklyn/core/entity/Entities.java | 1 -
.../stock/AbstractMultipleSensorAggregator.java | 9 +-
.../ApplicationLifecycleStateStressTest.java | 11 ++-
.../entity/ApplicationLifecycleStateTest.java | 99 ++++++++++++++------
4 files changed, 85 insertions(+), 35 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/16a44c25/core/src/main/java/org/apache/brooklyn/core/entity/Entities.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/entity/Entities.java b/core/src/main/java/org/apache/brooklyn/core/entity/Entities.java
index 02ea9b3..2e38a34 100644
--- a/core/src/main/java/org/apache/brooklyn/core/entity/Entities.java
+++ b/core/src/main/java/org/apache/brooklyn/core/entity/Entities.java
@@ -76,7 +76,6 @@ import org.apache.brooklyn.core.sensor.DependentConfiguration;
import org.apache.brooklyn.util.collections.MutableMap;
import org.apache.brooklyn.util.collections.MutableSet;
import org.apache.brooklyn.util.core.ResourceUtils;
-import org.apache.brooklyn.util.core.config.ConfigBag;
import org.apache.brooklyn.util.core.task.DynamicTasks;
import org.apache.brooklyn.util.core.task.ParallelTask;
import org.apache.brooklyn.util.core.task.TaskTags;
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/16a44c25/core/src/main/java/org/apache/brooklyn/enricher/stock/AbstractMultipleSensorAggregator.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/enricher/stock/AbstractMultipleSensorAggregator.java b/core/src/main/java/org/apache/brooklyn/enricher/stock/AbstractMultipleSensorAggregator.java
index 7de27ca..8249348 100644
--- a/core/src/main/java/org/apache/brooklyn/enricher/stock/AbstractMultipleSensorAggregator.java
+++ b/core/src/main/java/org/apache/brooklyn/enricher/stock/AbstractMultipleSensorAggregator.java
@@ -18,7 +18,6 @@
*/
package org.apache.brooklyn.enricher.stock;
-import java.util.Collection;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.Map;
@@ -102,7 +101,13 @@ public abstract class AbstractMultipleSensorAggregator<U> extends AbstractAggreg
initialVal = null;
}
vs.put(producer, initialVal != null ? initialVal : defaultMemberValue);
- // NB: see notes on possible race, in Aggregator#onProducerAdded
+ // there may be sensor events older than initialVal because this is run in
+ // the _parents_ synched subscription block. we live with this for now,
+ // as those sensor events will catch up, but val here might see
+ // second -> first -> second on child publishing second -> first .
+ // could resolve by running this init in the subscription manager thread;
+ // but it's not necessary yet; see Aggregator#onProducerAdded and
+ // ApplicationLifecycleStateTest#testSensorInitAndPublishOrder
}
}
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/16a44c25/core/src/test/java/org/apache/brooklyn/core/entity/ApplicationLifecycleStateStressTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/entity/ApplicationLifecycleStateStressTest.java b/core/src/test/java/org/apache/brooklyn/core/entity/ApplicationLifecycleStateStressTest.java
index aa1790d..581957a 100644
--- a/core/src/test/java/org/apache/brooklyn/core/entity/ApplicationLifecycleStateStressTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/entity/ApplicationLifecycleStateStressTest.java
@@ -26,6 +26,7 @@ public class ApplicationLifecycleStateStressTest extends ApplicationLifecycleSta
// TODO Is there a nicer way to get all the super-class public methods to run with the
// invocationCount defined on this class, rather than the simple `@Test` annotation on
// the super-class (without redefining all the public methods here!)?
+ // Alex replying: I don't think so, unless you possibly ran the "parent" as suite 100 times
@Override
public void testHappyPathEmptyApp() throws Exception {
@@ -87,10 +88,14 @@ public class ApplicationLifecycleStateStressTest extends ApplicationLifecycleSta
super.testSettingSensorFromThreads();
}
- @Test(groups="Broken")
@Override
- public void testWrongSensorInitValue() {
- super.testWrongSensorInitValue();
+ public void testSensorInitAndPublishOrder() {
+ super.testSensorInitAndPublishOrder();
+ }
+
+ @Override
+ public void testSensorInitAndPublishOrderNonStrict() {
+ super.testSensorInitAndPublishOrderNonStrict();
}
@Override
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/16a44c25/core/src/test/java/org/apache/brooklyn/core/entity/ApplicationLifecycleStateTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/entity/ApplicationLifecycleStateTest.java b/core/src/test/java/org/apache/brooklyn/core/entity/ApplicationLifecycleStateTest.java
index 470d0bf..4c5a09e 100644
--- a/core/src/test/java/org/apache/brooklyn/core/entity/ApplicationLifecycleStateTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/entity/ApplicationLifecycleStateTest.java
@@ -19,12 +19,10 @@
package org.apache.brooklyn.core.entity;
import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertTrue;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
-import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicReference;
@@ -57,6 +55,7 @@ import org.apache.brooklyn.util.collections.QuorumCheck;
import org.apache.brooklyn.util.core.task.ValueResolver;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import org.testng.Assert;
import org.testng.annotations.Test;
import com.google.common.base.Predicates;
@@ -65,7 +64,6 @@ import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
-import com.google.common.collect.Ordering;
@Test
public class ApplicationLifecycleStateTest extends BrooklynMgmtUnitTestSupport {
@@ -272,7 +270,8 @@ public class ApplicationLifecycleStateTest extends BrooklynMgmtUnitTestSupport {
public static class RecodingChildSensorEnricher extends AbstractMultipleSensorAggregator<Void> {
public static AttributeSensor<String> RECORDED_SENSOR = Sensors.newStringSensor("recorded.sensor");
- List<String> seenValues = new ArrayList<>();
+ List<String> seenValuesAllCollected = Collections.synchronizedList(new ArrayList<>());
+ List<String> seenValuesPublished = Collections.synchronizedList(new ArrayList<>());
@Override
protected Collection<Sensor<?>> getSourceSensors() {
@@ -289,44 +288,72 @@ public class ApplicationLifecycleStateTest extends BrooklynMgmtUnitTestSupport {
}
@Override
+ public void onEvent(SensorEvent<Object> event) {
+ super.onEvent(event);
+
+ if (event.getSensor().equals(RECORDED_SENSOR)) {
+ seenValuesPublished.add((String)event.getValue());
+ }
+ }
+
+ @Override
protected void onUpdated() {
- Iterator<String> values = getValues(RECORDED_SENSOR).values().iterator();
- if (values.hasNext()) {
- seenValues.add(values.next());
+ // called in onEvent, this is longwinded way to add the event.value we are interested in
+ Collection<String> vv = getValues(RECORDED_SENSOR).values();
+ if (!vv.isEmpty()) {
+ seenValuesAllCollected.add(vv.iterator().next());
}
}
}
- /**
- * Enricher sees state in the wrong order -> running, starting, running
- *
- * Indeterministic, fails a couple of times per 100 invocations when run with "mvn test" in the
- * brooklyn-itest docker container.
- */
- @Test(groups="Broken")
- public void testWrongSensorInitValue() {
+ @Test
+ public void testSensorInitAndPublishOrderNonStrict() {
+ // this mirrors how most code is working - the initial values seen for an added child may be later than other published values
+ doTestSensorInitAndPublishOrder(false);
+ }
+
+ @Test
+ public void testSensorInitAndPublishOrder() {
+ // this enforces canonical order by waiting on expected events
+ doTestSensorInitAndPublishOrder(true);
+ }
+
+ private void doTestSensorInitAndPublishOrder(boolean strict) {
TestApplication app = mgmt.getEntityManager().createEntity(EntitySpec.create(TestApplication.class)
.impl(TestApplicationNoEnrichersImpl.class)
.enricher(EnricherSpec.create(RecodingChildSensorEnricher.class))
.child(EntitySpec.create(TestEntity.class)
.impl(TestEntityNoEnrichersImpl.class)));
+
+ final RecodingChildSensorEnricher enricher = getFirstEnricher(app, RecodingChildSensorEnricher.class);
+ if (strict) {
+ Asserts.eventually(Suppliers.ofInstance(enricher.seenValuesAllCollected), CollectionFunctionals.sizeEquals(2));
+ }
Entity child = Iterables.get(app.getChildren(), 0);
child.sensors().set(RecodingChildSensorEnricher.RECORDED_SENSOR, "first");
child.sensors().set(RecodingChildSensorEnricher.RECORDED_SENSOR, "second");
-
- final RecodingChildSensorEnricher enricher = getFirstEnricher(app, RecodingChildSensorEnricher.class);
-
- // setEntity -> onUpdate
- // CHILD_ADDED -> onUpdate
- // set RECORDED_SENSOR=first -> onUpdate
- // set RECORDED_SENSOR=second -> onUpdate
- Asserts.eventually(Suppliers.ofInstance(enricher.seenValues), CollectionFunctionals.sizeEquals(4));
-
- boolean isOrdered = Ordering.explicit(MutableList.of("first", "second"))
- .nullsFirst()
- .isOrdered(enricher.seenValues);
- assertTrue(isOrdered, "Unexpected ordering for " + enricher.seenValues);
+
+ // usually:
+ // setEntity -> onUpdate (sees current value)
+ // CHILD_ADDED -> onUpdate (sees current value)
+ // set RECORDED_SENSOR=first -> onUpdate (sees event)
+ // set RECORDED_SENSOR=second -> onUpdate (sees event)
+
+ Asserts.eventually(Suppliers.ofInstance(enricher.seenValuesAllCollected), CollectionFunctionals.sizeEquals(4));
+
+ Assert.assertEquals(enricher.seenValuesPublished, MutableList.of("first", "second"),
+ "Unexpected ordering for vP " + enricher.seenValuesPublished + " (vC "+enricher.seenValuesAllCollected+")");
+
+ List<String> vC = MutableList.copyOf(enricher.seenValuesAllCollected);
+ if (!strict) {
+ // CHILD_ADDED (and other handlers) are running in parallel to this thread, however,
+ // their order is guaranteed to be the above, but it is in parallel to this main thread, thus the current value
+ // it takes can be any of null, first, or second, unless we run in strict mode
+ vC.set(1, null);
+ }
+ Assert.assertEquals(vC, MutableList.of(null, null, "first", "second"),
+ "Unexpected ordering for vC " + enricher.seenValuesAllCollected + " (vP "+enricher.seenValuesPublished+")");
}
public static class EmittingEnricher extends AbstractEnricher {
@@ -428,5 +455,19 @@ public class ApplicationLifecycleStateTest extends BrooklynMgmtUnitTestSupport {
.first()
.get();
}
-
+
+ // for running the tests below many times and stopping when they fail;
+ // they seem happy (and this breaks the subclass @Test class annotation)
+ // but kept as comment just in case
+// public static void main(String[] args) throws Exception {
+// int i = 0;
+// while (true) {
+// log.info("test "+(++i));
+// ApplicationLifecycleStateTest l = new ApplicationLifecycleStateTest();
+// l.setUp();
+// l.testSensorInitAndPublishOrder();
+// l.testSensorInitAndPublishOrderNonStrict();
+// l.tearDown();
+// }
+// }
}