You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by googlielmo <gi...@git.apache.org> on 2015/12/22 16:56:26 UTC
[GitHub] incubator-brooklyn pull request: Test correct persistence of attri...
GitHub user googlielmo opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/1120
Test correct persistence of attributeWhenReady
An entity with an instance of `attributeWhenReady` is serialised and then deserialised
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/googlielmo/incubator-brooklyn feature/deferred-supplier-persistence
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/1120.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1120
----
commit a76c44fad4f9577848c604b3c93a29f3f8a231a1
Author: Guglielmo Nigri <gu...@cloudsoftcorp.com>
Date: 2015-12-22T15:43:22Z
Test correct persistence of attributeWhenReady
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Test correct persistence of attri...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1120#discussion_r48551193
--- Diff: brooklyn-server/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/DslAndRebindYamlTest.java ---
@@ -83,15 +95,14 @@ protected Logger getLogger() {
public Application rebind(Application app) throws Exception {
RebindTestUtils.waitForPersisted(app);
- // not strictly needed, but for good measure:
- RebindTestUtils.checkCurrentMementoSerializable(app);
+ // Removed because of issues in some tests: // RebindTestUtils.checkCurrentMementoSerializable(app);
--- End diff --
Fine to just delete this line entirely, I think.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Test correct persistence of attri...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/1120#issuecomment-167899413
See suggested additional tests, and alterations to tests, in https://github.com/googlielmo/incubator-brooklyn/pull/2
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Test correct persistence of attri...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/1120
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Test correct persistence of attri...
Posted by googlielmo <gi...@git.apache.org>.
Github user googlielmo commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/1120#issuecomment-167605858
@ahgittin I turned it into a slightly more complicated test that serialises and then deserialises an entity **spec** while a `Task` is waiting for the attribute value to be available. This is to simulate a situation that may happen frequently at runtime. Previously we only tested AWR persistence with no other threads in wait.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Test correct persistence of attri...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1120#discussion_r48564756
--- Diff: brooklyn-server/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/DslAndRebindYamlTest.java ---
@@ -115,34 +125,207 @@ public T call() throws Exception {
}
}).build()).getUnchecked();
}
-
+
@Test
public void testDslAttributeWhenReady() throws Exception {
Entity testEntity = entityWithAttributeWhenReady();
- ((EntityInternal)testEntity).sensors().set(Sensors.newStringSensor("foo"), "bar");
+ ((EntityInternal) testEntity).sensors().set(Sensors.newStringSensor("foo"), "bar");
Assert.assertEquals(getConfigInTask(testEntity, TestEntity.CONF_NAME), "bar");
}
@Test
+ public void testDslAttributeWhenReadyPersisted() throws Exception {
+ Entity testEntity = entityWithAttributeWhenReady();
+
+ // Persist and rebind
+ Application app2 = rebind(testEntity.getApplication());
+ Entity e2 = Iterables.getOnlyElement(app2.getChildren());
+
+ Maybe<Object> maybe = ((EntityInternal) e2).config().getLocalRaw(TestEntity.CONF_NAME);
+ Assert.assertTrue(maybe.isPresentAndNonNull());
+ Assert.assertTrue(BrooklynDslDeferredSupplier.class.isInstance(maybe.get()));
+ BrooklynDslDeferredSupplier deferredSupplier = (BrooklynDslDeferredSupplier) maybe.get();
+ Assert.assertEquals(deferredSupplier.toString(), "$brooklyn:entity(\"x\").attributeWhenReady(\"foo\")");
+
+ // Assert the persisted state itself is as expected, and not too big
+ BrooklynMementoRawData raw = BrooklynPersistenceUtils.newStateMemento(app2.getManagementContext(), MementoCopyMode.LOCAL);
+ String persistedStateForE2 = raw.getEntities().get(e2.getId());
+ Matcher matcher = Pattern.compile(".*\\<test.confName\\>(.*)\\<\\/test.confName\\>.*", Pattern.DOTALL)
+ .matcher(persistedStateForE2);
+ Assert.assertTrue(matcher.find());
+ String testConfNamePersistedState = matcher.group(1);
+
+ Assert.assertNotNull(testConfNamePersistedState);
+ // should be about 200 chars long, something like:
+ //
+ // <test.confName>
+ // <org.apache.brooklyn.camp.brooklyn.spi.dsl.methods.DslComponent_-AttributeWhenReady>
+ // <component>
+ // <componentId>x</componentId>
+ // <scope>GLOBAL</scope>
+ // </component>
+ // <sensorName>foo</sensorName>
+ // </org.apache.brooklyn.camp.brooklyn.spi.dsl.methods.DslComponent_-AttributeWhenReady>
+ // </test.confName>
+
+ Assert.assertTrue(testConfNamePersistedState.length() < 400, "persisted state too long: " + testConfNamePersistedState);
+ }
+
+ @Test
+ public void testDslAttributeWhenReadyPersistedInEntitySpecWhileTaskIsWaiting() throws Exception {
+ String yaml = "location: localhost\n" +
+ "name: Test Cluster\n" +
+ "services:\n" +
+ "- type: org.apache.brooklyn.entity.group.DynamicCluster\n" +
+ " id: test-cluster\n" +
+ " initialSize: 1\n" +
+ " memberSpec:\n" +
+ " $brooklyn:entitySpec:\n" +
+ " type: org.apache.brooklyn.core.test.entity.TestEntity\n" +
+ " brooklyn.config:\n" +
+ " test.confName: $brooklyn:component(\"test-cluster\").attributeWhenReady(\"sensor\")";
+
+ final Entity testEntity = createAndStartApplication(yaml);
+
+ DynamicCluster clusterEntity1 = (DynamicCluster) Iterables.getOnlyElement(testEntity.getApplication().getChildren());
+
+ TestEntity testEntity1 = null;
+ for (Entity entity : clusterEntity1.getChildren()) {
+ if (entity instanceof TestEntity) {
+ testEntity1 = (TestEntity) entity;
+ break;
+ }
+ }
+ Assert.assertNotNull(testEntity1, "TestEntity not found in DynamicCluster");
+
+ final TestEntity childTestEntity = testEntity1;
+
+ // Wait for the attribute to be ready in a new Task
+ Callable<String> configGetter = new Callable<String>() {
+ @Override
+ public String call() throws Exception {
+ String s = getConfigInTask(childTestEntity, TestEntity.CONF_NAME);
+ getLogger().info("getConfig {}={}", TestEntity.CONF_NAME, s);
+ return s;
+ }
+ };
+ ExecutorService executorService = Executors.newSingleThreadExecutor();
+ Future<String> stringFuture = executorService.submit(configGetter);
+
+ // Persist and rebind
+ Application app2 = rebind(testEntity.getApplication());
+
+ DynamicCluster clusterEntity2 = (DynamicCluster) Iterables.getOnlyElement(app2.getApplication().getChildren());
+
+ TestEntity testEntity2 = null;
+ for (Entity entity : clusterEntity2.getChildren()) {
+ if (entity instanceof TestEntity) {
+ testEntity2 = (TestEntity) entity;
+ break;
+ }
+ }
+ Assert.assertNotNull(testEntity2, "TestEntity not found in DynamicCluster");
+
+ Maybe<Object> maybe = testEntity2.config().getLocalRaw(TestEntity.CONF_NAME);
+
+ Assert.assertTrue(maybe.isPresentAndNonNull());
+ Assert.assertTrue(BrooklynDslDeferredSupplier.class.isInstance(maybe.get()));
+ BrooklynDslDeferredSupplier deferredSupplier = (BrooklynDslDeferredSupplier) maybe.get();
+ Assert.assertEquals(deferredSupplier.toString(), "$brooklyn:entity(\"test-cluster\").attributeWhenReady(\"sensor\")");
+
+ // Check that the Task is still waiting for attribute to be ready
+ Assert.assertFalse(stringFuture.isDone());
+
+ // Now set sensor value
+ ((EntityInternal) clusterEntity1).sensors().set(Sensors.newStringSensor("sensor"), "bar");
--- End diff --
This seems to be confusing modifying the entities from before rebind, but doing so after rebind. The call to `rebind()` is to simulate us having restarted the Brooklyn server. It's configurable whether we actually terminate the original management context. I guess this test is passing because the original management context was not terminated.
I'm looking at these tests in more detail. I'll make a few changes and submit a pull request against your branch @googlielmo (either this evening or tomorrow).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Test correct persistence of attri...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1120#discussion_r48272538
--- Diff: usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/DslAndRebindYamlTest.java ---
@@ -124,6 +126,19 @@ public void testDslAttributeWhenReady() throws Exception {
}
@Test
+ public void testDslAttributeWhenReadyPersisted() throws Exception {
--- End diff --
as discussed we want to test where `attributeWhenReady` is used in an entity spec, e.g. something like
```
name: Test Cluster
services:
- type: org.apache.brooklyn.entity.stock.DynamicCluster
id: test-cluster
initialSize: 1
memberSpec:
$brooklyn:entitySpec:
type: org.apache.brooklyn.entity.test.TestEntity
brooklyn.config:
test.confName: $brooklyn:component("test-cluster").attributeWhenReady("sensor")
```
with this you could then in your test set `test-cluster` sensor `sensor` e.g. to `foo` and then ensure that `foo` is *not* being written (ie not in the `testConfNamePersistedState`)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Test correct persistence of attri...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1120#discussion_r48335258
--- Diff: usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/DslAndRebindYamlTest.java ---
@@ -124,6 +126,19 @@ public void testDslAttributeWhenReady() throws Exception {
}
@Test
+ public void testDslAttributeWhenReadyPersisted() throws Exception {
--- End diff --
For the `java.io.NotSerializableException`: that's surprising. We shouldn't be relying on normal java serialization. We use xstream for persistence, which doesn't need `Serializable`.
Some test assertions can enable a stronger assertion about being able to serialize all of the persisted state using java serialization (in an attempt to fail-fast). We can just disable that, if it's being done. It's too strict.
Worth looking down the stacktrace to see what is trying to do the java serialization, to see if it is the test assertion or something else.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Test correct persistence of attri...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1120#discussion_r48272017
--- Diff: usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/DslAndRebindYamlTest.java ---
@@ -124,6 +126,19 @@ public void testDslAttributeWhenReady() throws Exception {
}
@Test
+ public void testDslAttributeWhenReadyPersisted() throws Exception {
+ Entity testEntity = entityWithAttributeWhenReady();
+ Application app2 = rebind(testEntity.getApplication());
+ Entity e2 = Iterables.getOnlyElement( app2.getChildren() );
+
+ Maybe<Object> maybe = ((EntityInternal)e2).config().getLocalRaw(TestEntity.CONF_NAME);
+ Assert.assertTrue(maybe.isPresentAndNonNull());
+ Assert.assertTrue(BrooklynDslDeferredSupplier.class.isInstance(maybe.get()));
+ BrooklynDslDeferredSupplier deferredSupplier = (BrooklynDslDeferredSupplier) maybe.get();
+ Assert.assertEquals(deferredSupplier.toString(), "$brooklyn:entity(\"x\").attributeWhenReady(\"foo\")");
+ }
--- End diff --
suggest also adding something like this:
```
BrooklynMementoRawData raw = BrooklynPersistenceUtils.newStateMemento(app2.getManagementContext(), MementoCopyMode.LOCAL);
String persistedStateForE2 = raw.getEntities().get(e2.getId());
Matcher matcher = Pattern.compile(".*\\<test.confName\\>(.*)\\<\\/test.confName\\>.*", Pattern.DOTALL)
.matcher(persistedStateForE2);
Assert.assertTrue(matcher.find());
String testConfNamePersistedState = matcher.group(1);
Assert.assertNotNull(testConfNamePersistedState);
// should be about 200 chars long...
Assert.assertTrue(testConfNamePersistedState.length() < 400, "persisted state too long: "+testConfNamePersistedState);
```
this asserts the persisted state is as expected, and not too big. btw this is what it looks like:
```
// <test.confName>
// <org.apache.brooklyn.camp.brooklyn.spi.dsl.methods.DslComponent_-AttributeWhenReady>
// <component>
// <componentId>x</componentId>
// <scope>GLOBAL</scope>
// </component>
// <sensorName>foo</sensorName>
// </org.apache.brooklyn.camp.brooklyn.spi.dsl.methods.DslComponent_-AttributeWhenReady>
// </test.confName>
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Test correct persistence of attri...
Posted by googlielmo <gi...@git.apache.org>.
Github user googlielmo commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1120#discussion_r48491855
--- Diff: usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/DslAndRebindYamlTest.java ---
@@ -124,6 +126,19 @@ public void testDslAttributeWhenReady() throws Exception {
}
@Test
+ public void testDslAttributeWhenReadyPersisted() throws Exception {
--- End diff --
@aledsage you are right! it was due to a stronger test assertion -- I disabled it for now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Test correct persistence of attri...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1120#discussion_r48873785
--- Diff: brooklyn-server/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/DslAndRebindYamlTest.java ---
@@ -19,58 +19,82 @@
package org.apache.brooklyn.camp.brooklyn;
import java.io.File;
+import java.util.List;
import java.util.Set;
import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import org.apache.brooklyn.api.entity.Application;
import org.apache.brooklyn.api.entity.Entity;
import org.apache.brooklyn.api.mgmt.ManagementContext;
+import org.apache.brooklyn.api.mgmt.ha.MementoCopyMode;
+import org.apache.brooklyn.api.mgmt.rebind.mementos.BrooklynMementoRawData;
import org.apache.brooklyn.api.sensor.AttributeSensor;
import org.apache.brooklyn.api.sensor.Sensor;
+import org.apache.brooklyn.camp.brooklyn.spi.dsl.BrooklynDslDeferredSupplier;
import org.apache.brooklyn.config.ConfigKey;
import org.apache.brooklyn.core.config.ConfigKeys;
import org.apache.brooklyn.core.entity.Attributes;
import org.apache.brooklyn.core.entity.Entities;
+import org.apache.brooklyn.core.entity.EntityAsserts;
import org.apache.brooklyn.core.entity.EntityInternal;
import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext;
+import org.apache.brooklyn.core.mgmt.persist.BrooklynPersistenceUtils;
import org.apache.brooklyn.core.mgmt.rebind.RebindTestUtils;
import org.apache.brooklyn.core.sensor.Sensors;
import org.apache.brooklyn.core.test.entity.TestEntity;
-import org.apache.brooklyn.test.EntityTestUtils;
+import org.apache.brooklyn.entity.group.DynamicCluster;
import org.apache.brooklyn.util.collections.MutableSet;
import org.apache.brooklyn.util.core.task.Tasks;
+import org.apache.brooklyn.util.guava.Maybe;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.testng.Assert;
import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
import com.google.common.io.Files;
@Test
public class DslAndRebindYamlTest extends AbstractYamlTest {
-
+
private static final Logger log = LoggerFactory.getLogger(DslAndRebindYamlTest.class);
-
+
protected ClassLoader classLoader = getClass().getClassLoader();
protected File mementoDir;
protected Set<ManagementContext> mgmtContexts = MutableSet.of();
+ protected ExecutorService executor;
@Override
protected LocalManagementContext newTestManagementContext() {
- if (mementoDir!=null) throw new IllegalStateException("already created mgmt context");
+ if (mementoDir != null) throw new IllegalStateException("already created mgmt context");
mementoDir = Files.createTempDir();
mementoDir.deleteOnExit();
LocalManagementContext mgmt = RebindTestUtils.newPersistingManagementContext(mementoDir, classLoader, 1);
mgmtContexts.add(mgmt);
return mgmt;
}
+
+ @BeforeMethod(alwaysRun = true)
+ @Override
+ public void setUp() {
+ super.setUp();
--- End diff --
Are these tabs? Or 3 spaces? Can you change them to 4 spaces please?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Test correct persistence of attri...
Posted by googlielmo <gi...@git.apache.org>.
Github user googlielmo commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1120#discussion_r48301440
--- Diff: usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/DslAndRebindYamlTest.java ---
@@ -124,6 +126,19 @@ public void testDslAttributeWhenReady() throws Exception {
}
@Test
+ public void testDslAttributeWhenReadyPersisted() throws Exception {
--- End diff --
I set up a test with that (see below), except that I now get `java.io.NotSerializableException: org.apache.brooklyn.entity.group.DynamicClusterImpl$NextClusterMemberIdSupplier`
Will look into it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Test correct persistence of attri...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/1120#issuecomment-169709323
Thanks @googlielmo - merging now (and will check about tabs versus spaces after that).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---