You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by aledsage <gi...@git.apache.org> on 2014/11/13 14:32:59 UTC
[GitHub] incubator-brooklyn pull request: Fix various tests
GitHub user aledsage opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/326
Fix various tests
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/aledsage/incubator-brooklyn fix/various-tests-20141113
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/326.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 #326
----
commit b77421f8c7b2481b22acd04f1ce9aa368ac5e280
Author: Aled Sage <al...@gmail.com>
Date: 2014-11-13T10:55:21Z
Fix ApplicationResourceIntegrationTest
- can get back json or plain string from
client().resource(stateSensor).get(String.class);
accept either
commit 3a34862786fd1be8f9d0fe609fec9fa8cef5c0d6
Author: Aled Sage <al...@gmail.com>
Date: 2014-11-13T10:56:17Z
Fix MemoryUsageTrackerTest
- was non-deterministic (asserting memory usage immediately, before
listener had been called by GC)
commit 956514e2986a79b3e0b2faafecedf4d42c636e5a
Author: Aled Sage <al...@gmail.com>
Date: 2014-11-13T11:19:34Z
Fix feed rebind
- Was expecting xpath /feed/id, but was
/brooklyn.entity.rebind.dto.BasicFeedMemento/id
- Changed XmlMementoSerializer to use “feed”
- Integration test failing was NginxRebindWithHaIntegrationTest.testChangeModeFailureStopsTasksButHappyUponResumption
commit ce6744d7f523b553a5b6cf71c00d0f5cb3733b1d
Author: Aled Sage <al...@gmail.com>
Date: 2014-11-13T12:26:37Z
Fix yaml use of SshCommandSensor
- Was causing failure of VanillaBashNetcatYamlTest
- SshCommandSensor constructor expects ConfigBag, rather than Map.
- Change BrooklynYamlTypeInstantiator.newInstance to look for a
<init>(ConfigBag) constructor as well.
- Fix `Class AddSensor.getType("String")` so it tries prepending
"java.lang." as well
- set poll period to 100ms in yaml (instead of default 5 minutes)
commit b1ca85f61627e0fc4b7f86e31c1056ceb3914be2
Author: Aled Sage <al...@gmail.com>
Date: 2014-11-13T12:48:59Z
Fix SshCommandIntegrationTest
- Change sensor period to 100ms, from default 5 mins.
Otherwise if first poll happens before serviceUp=true, it will
timeout in 30 seconds without a second poll.
commit 3fd9046ec0143af68f4101529402595dae5f3dd4
Author: Aled Sage <al...@gmail.com>
Date: 2014-11-13T12:50:40Z
Fix HttpRequestSensorTest.testHttpSensor
- Change sensor period to 100ms, from default 5 mins.
Otherwise if first poll happens before serviceUp=true, it will
timeout in 30 seconds without a second poll.
commit 7d315fd6774f5cb56e9e9a93bf644ffd36f6c0c3
Author: Aled Sage <al...@gmail.com>
Date: 2014-11-13T12:58:55Z
BROOKLYN-83: fix SensorResourceTest
- On getting a sensor, if no "Accepts" header is given, then we don't
control whether json or plain text comes back.
It is dependent on the method order, which is compiler-specific.
- Therefore delete the test that gave no Accepts and then asserted
it was json.
commit 3f4328c625fbcbb66eef8b9883a15b06530d8902
Author: Aled Sage <al...@gmail.com>
Date: 2014-11-13T13:19:51Z
BROOKLYN-85: fix FixedListMachineProvisioningLocationTest
----
---
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: Fix various tests
Posted by alasdairhodge <gi...@git.apache.org>.
Github user alasdairhodge commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/326#discussion_r20293897
--- Diff: core/src/main/java/brooklyn/entity/effector/AddSensor.java ---
@@ -76,14 +76,22 @@ public void apply(EntityLocal entity) {
return Sensors.newSensor(clazz, name);
}
+ @SuppressWarnings("unchecked")
protected Class<T> getType(String className) {
- Class<T> clazz = null;
try {
- clazz = (Class<T>) Class.forName(className);
+ return (Class<T>) Class.forName(className);
} catch (ClassNotFoundException e) {
- throw new IllegalArgumentException("Invalid target type for sensor "+name+": " + className);
+ if (!className.contains(".")) {
+ // could be assuming "java.lang" package; try again with that
--- End diff --
Who does this?! Requiring FQCNs isn't unreasonable.
---
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: Fix various tests
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/326#discussion_r20328811
--- Diff: usage/rest-server/src/test/java/brooklyn/rest/resources/ApplicationResourceIntegrationTest.java ---
@@ -101,16 +101,19 @@ public boolean apply(SensorSummary sensorSummary) {
}
@Test(groups="Integration", dependsOnMethods = { "testListSensorsRedis", "testListEntities" })
- public void testTriggerRedisStopEffector() throws InterruptedException {
+ public void testTriggerRedisStopEffector() throws Exception {
ClientResponse response = client().resource("/v1/applications/redis-app/entities/redis-ent/effectors/stop")
.type(MediaType.APPLICATION_JSON_TYPE)
.post(ClientResponse.class, ImmutableMap.of());
assertEquals(response.getStatus(), Response.Status.ACCEPTED.getStatusCode());
final URI stateSensor = URI.create("/v1/applications/redis-app/entities/redis-ent/sensors/service.state");
- final String expectedStatus = String.format("\"%s\"", Lifecycle.STOPPED.toString());
+ final String expectedStatus = Lifecycle.STOPPED.toString();
Asserts.succeedsEventually(MutableMap.of("timeout", 60 * 1000), new Runnable() {
public void run() {
+ // Accept with and without quotes; if don't specify "Accepts" header, then
+ // might get back json or plain text (depending on compiler / java runtime
+ // used for SensorApi!)
--- End diff --
See https://issues.apache.org/jira/browse/BROOKLYN-83 for more discussion.
I'm not sure how we bend resteasy to our will for that, especially given https://docs.oracle.com/cd/E19776-01/820-4867/ghrpv/ saying that it is the first method.
If there is no "Accepts" passed in, then how would we ensure that only one of the methods matches?
Anyway, not addressing in this PR.
---
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: Fix various tests
Posted by alasdairhodge <gi...@git.apache.org>.
Github user alasdairhodge commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/326#discussion_r20294956
--- Diff: utils/common/src/test/java/brooklyn/util/javalang/MemoryUsageTrackerTest.java ---
@@ -20,35 +20,54 @@
import java.util.List;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import org.testng.Assert;
import org.testng.annotations.Test;
+import brooklyn.test.Asserts;
import brooklyn.util.collections.MutableList;
import brooklyn.util.guava.Maybe;
import brooklyn.util.text.Strings;
public class MemoryUsageTrackerTest {
+ private static final Logger LOG = LoggerFactory.getLogger(MemoryUsageTrackerTest.class);
+
@Test(groups="Integration")
public void testBigUsage() {
+ // Don't just use runtime.maxMemory()*2; javadoc says:
+ // If there is no inherent limit then the value java.lang.Long.MAX_VALUE will be returned.
+ // Therefore cap at 10GB.
+ final long maxMemory = Math.min(Runtime.getRuntime().maxMemory(), 10*1024*1024*1024L);
+
List<Maybe<byte[]>> references = MutableList.of();
long created = 0;
- while (created < 2*Runtime.getRuntime().maxMemory()) {
- byte d[] = new byte[1000000];
+ while (created < 2*maxMemory) {
+ byte d[] = new byte[10*1000*1000];
--- End diff --
Nitpick: a named constant might be nice; `MAX_MEMORY_CAP`?
---
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: Fix various tests
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/326#issuecomment-62980453
Ah! @ahgittin got in just before me with merging https://github.com/apache/incubator-brooklyn/pull/312, causing merge conflicts here. I'll resolve those and then merge.
---
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: Fix various tests
Posted by alasdairhodge <gi...@git.apache.org>.
Github user alasdairhodge commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/326#discussion_r20295024
--- Diff: utils/common/src/test/java/brooklyn/util/javalang/MemoryUsageTrackerTest.java ---
@@ -20,35 +20,54 @@
import java.util.List;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import org.testng.Assert;
import org.testng.annotations.Test;
+import brooklyn.test.Asserts;
import brooklyn.util.collections.MutableList;
import brooklyn.util.guava.Maybe;
import brooklyn.util.text.Strings;
public class MemoryUsageTrackerTest {
+ private static final Logger LOG = LoggerFactory.getLogger(MemoryUsageTrackerTest.class);
+
@Test(groups="Integration")
public void testBigUsage() {
+ // Don't just use runtime.maxMemory()*2; javadoc says:
+ // If there is no inherent limit then the value java.lang.Long.MAX_VALUE will be returned.
+ // Therefore cap at 10GB.
+ final long maxMemory = Math.min(Runtime.getRuntime().maxMemory(), 10*1024*1024*1024L);
+
List<Maybe<byte[]>> references = MutableList.of();
long created = 0;
- while (created < 2*Runtime.getRuntime().maxMemory()) {
- byte d[] = new byte[1000000];
+ while (created < 2*maxMemory) {
+ byte d[] = new byte[10*1000*1000];
--- End diff --
Nitpick: another contender for named constant; `ALLOCATION_CHUNK_SIZE`?
---
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: Fix various tests
Posted by alasdairhodge <gi...@git.apache.org>.
Github user alasdairhodge commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/326#discussion_r20294992
--- Diff: utils/common/src/test/java/brooklyn/util/javalang/MemoryUsageTrackerTest.java ---
@@ -20,35 +20,54 @@
import java.util.List;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import org.testng.Assert;
import org.testng.annotations.Test;
+import brooklyn.test.Asserts;
import brooklyn.util.collections.MutableList;
import brooklyn.util.guava.Maybe;
import brooklyn.util.text.Strings;
public class MemoryUsageTrackerTest {
+ private static final Logger LOG = LoggerFactory.getLogger(MemoryUsageTrackerTest.class);
+
@Test(groups="Integration")
public void testBigUsage() {
+ // Don't just use runtime.maxMemory()*2; javadoc says:
+ // If there is no inherent limit then the value java.lang.Long.MAX_VALUE will be returned.
+ // Therefore cap at 10GB.
+ final long maxMemory = Math.min(Runtime.getRuntime().maxMemory(), 10*1024*1024*1024L);
--- End diff --
Nitpick: a named constant might be nice here; `MAX_MEMORY_CAP`?
---
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: Fix various tests
Posted by alasdairhodge <gi...@git.apache.org>.
Github user alasdairhodge commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/326#issuecomment-62900747
Looks great.
---
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: Fix various tests
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/326#discussion_r20329212
--- Diff: core/src/main/java/brooklyn/entity/effector/AddSensor.java ---
@@ -76,14 +76,22 @@ public void apply(EntityLocal entity) {
return Sensors.newSensor(clazz, name);
}
+ @SuppressWarnings("unchecked")
protected Class<T> getType(String className) {
- Class<T> clazz = null;
try {
- clazz = (Class<T>) Class.forName(className);
+ return (Class<T>) Class.forName(className);
} catch (ClassNotFoundException e) {
- throw new IllegalArgumentException("Invalid target type for sensor "+name+": " + className);
+ if (!className.contains(".")) {
+ // could be assuming "java.lang" package; try again with that
--- End diff --
This allows "String" to be specified in yaml, rather than having to use `java.lang.String`, when specifying the sensor type being added.
I've changed the default to be `java.lang.String`, but will leave in the ugly check as folk will expect unqualified java.lang classes to "just work".
---
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: Fix various tests
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/326
---
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: Fix various tests
Posted by alasdairhodge <gi...@git.apache.org>.
Github user alasdairhodge commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/326#discussion_r20293349
--- Diff: usage/rest-server/src/test/java/brooklyn/rest/resources/ApplicationResourceIntegrationTest.java ---
@@ -101,16 +101,19 @@ public boolean apply(SensorSummary sensorSummary) {
}
@Test(groups="Integration", dependsOnMethods = { "testListSensorsRedis", "testListEntities" })
- public void testTriggerRedisStopEffector() throws InterruptedException {
+ public void testTriggerRedisStopEffector() throws Exception {
ClientResponse response = client().resource("/v1/applications/redis-app/entities/redis-ent/effectors/stop")
.type(MediaType.APPLICATION_JSON_TYPE)
.post(ClientResponse.class, ImmutableMap.of());
assertEquals(response.getStatus(), Response.Status.ACCEPTED.getStatusCode());
final URI stateSensor = URI.create("/v1/applications/redis-app/entities/redis-ent/sensors/service.state");
- final String expectedStatus = String.format("\"%s\"", Lifecycle.STOPPED.toString());
+ final String expectedStatus = Lifecycle.STOPPED.toString();
Asserts.succeedsEventually(MutableMap.of("timeout", 60 * 1000), new Runnable() {
public void run() {
+ // Accept with and without quotes; if don't specify "Accepts" header, then
+ // might get back json or plain text (depending on compiler / java runtime
+ // used for SensorApi!)
--- End diff --
I've seen the discussion about this; it's horrible. Surely we should be able to guarantee deterministic behaviour?!
---
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: Fix various tests
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/326#issuecomment-62979836
Thanks @alasdairhodge - have incorporated comments (and rebased to merge into existing commits); will merge 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.
---