You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by neykov <gi...@git.apache.org> on 2016/11/07 17:13:30 UTC
[GitHub] brooklyn-server pull request #412: Fix integration tests in brooklyn-core
GitHub user neykov opened a pull request:
https://github.com/apache/brooklyn-server/pull/412
Fix integration tests in brooklyn-core
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/neykov/brooklyn-server fix/integration-tests-core
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/412.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 #412
----
commit c9f0af9ee56909eda10b5374d22d53a2034ba8fe
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date: 2016-11-07T17:13:00Z
Fix integration tests in brooklyn-core
----
---
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] brooklyn-server pull request #412: Fix integration tests in brooklyn-core
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/412#discussion_r86834710
--- Diff: core/src/test/java/org/apache/brooklyn/enricher/stock/EnrichersTest.java ---
@@ -201,17 +201,16 @@ public void testTransformingRespectsUnchangedButWillRepublish() {
entity.sensors().set(STR1, "myval");
Asserts.eventually(Suppliers.ofInstance(record), CollectionFunctionals.sizeEquals(1));
- EntityAsserts.assertAttributeEquals(entity, STR2, "myval");
+ EntityAsserts.assertAttributeEqualsEventually(entity, STR2, "myval");
--- End diff --
This test still fails for me. When running on an overloaded machine (I was compiling + running all the brooklyn-server tests concurrently!), it failed in a couple of places:
* The first `Asserts.assertThat(record.getEvents(), CollectionFunctionals.sizeEquals(0))` with the error
```
java.lang.AssertionError: Failed IsEqualTo(0)(sizeFunction): [TestEntityImpl{id=wcvg8rr83b}.Sensor: test.str2 (java.lang.String)=null @ 1478542432319]
```
* The final assertion `Asserts.eventually(Suppliers.ofInstance(record), CollectionFunctionals.sizeEquals(5));` - it had 6 items; the error was:
```
java.lang.AssertionError: Expected: eventually IsEqualTo(5)(sizeFunction); got most recently: RecordingSensorEventListener[size=6; events=[TestEntityImpl{id=gybou1giff}.Sensor: test.str2 (java.lang.String)=null @ 1478542608719, TestEntityImpl{id=gybou1giff}.Sensor: test.str2 (java.lang.String)=myval @ 1478542608725, TestEntityImpl{id=gybou1giff}.Sensor: test.str2 (java.lang.String)=myval2 @ 1478542609735, TestEntityImpl{id=gybou1giff}.Sensor: test.str2 (java.lang.String)=myval2 @ 1478542609739, TestEntityImpl{id=gybou1giff}.Sensor: test.str2 (java.lang.String)=myval2 @ 1478542609741, TestEntityImpl{id=gybou1giff}.Sensor: test.str2 (java.lang.String)=myval3 @ 1478542609744]] (waited 1s 5ms 196us, checked 86)
```
---
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] brooklyn-server pull request #412: Fix integration tests in brooklyn-server
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/412#discussion_r87561963
--- Diff: software/base/src/test/java/org/apache/brooklyn/entity/software/base/VanillaSoftwareProcessStreamsIntegrationTest.java ---
@@ -37,7 +37,10 @@ public void setUp() throws Exception {
localhost = app.getManagementContext().getLocationRegistry().getLocationManaged("localhost");
}
- @Test(groups = "Integration")
+ // Fails on subsequent runs because "BROOKLYN" marker already created in
--- End diff --
This can also be fixed by adding:
```
.configure(VanillaSoftwareProcess.INSTALL_UNIQUE_LABEL, Identifiers.makeRandomId(8))
```
---
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] brooklyn-server pull request #412: Fix integration tests in brooklyn-server
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/412
---
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] brooklyn-server pull request #412: Fix integration tests in brooklyn-core
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/412#discussion_r87418598
--- Diff: utils/common/src/test/java/org/apache/brooklyn/util/javalang/MemoryUsageTrackerTest.java ---
@@ -82,7 +82,7 @@ private long sizeOfActiveReferences(List<Maybe<byte[]>> references) {
return size;
}
- @Test(groups="Integration")
+ @Test(groups="Live")
--- End diff --
See https://github.com/apache/brooklyn-server/pull/426. Let's see if that fixes it. If you want to disable the test in the meantime, that's fine with me as well.
---
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] brooklyn-server issue #412: Fix integration tests in brooklyn-core
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/412
@aledsage Addressed comments plus some additional changes.
Agree we should increase the `eventually` timeout.
---
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] brooklyn-server pull request #412: Fix integration tests in brooklyn-server
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/412#discussion_r87574374
--- Diff: software/base/src/test/java/org/apache/brooklyn/entity/brooklynnode/BrooklynNodeIntegrationTest.java ---
@@ -578,7 +578,7 @@ public void testUsesLocation() throws Exception {
assertEquals(parseJson(locatedLocationsContent, ImmutableList.of(appLocationId, "longitude"), Double.class), 45.6, 0.00001);
}
- @Test(groups="Integration")
+ @Test(groups={"Integration", "Broken"})
--- End diff --
Didn't investigate. I expected that `BrooklynNode` is broken, but that's probably when using it with Karaf.
I'll have another look if it's working for you.
---
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] brooklyn-server pull request #412: Fix integration tests in brooklyn-server
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/412#discussion_r87560722
--- Diff: core/src/test/java/org/apache/brooklyn/core/entity/AbstractApplicationLegacyTest.java ---
@@ -92,8 +94,11 @@ public void testStartAndStopUnmanagedAppAutomanagesTheAppAndChildren() throws Ex
assertTrue(Entities.isManaged(app2));
assertTrue(Entities.isManaged(child));
assertEquals(child.getCallHistory(), ImmutableList.of("start"));
- assertEquals(mgmt.getEntityManager().getEntity(app2.getId()), app2);
--- End diff --
Wow, that's a really interesting error (that it used to pass because of `AbstractEntity.equals` matching on `o == selfProxy`, because `this.selfProxy` was null!). Well found and fixed!
---
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] brooklyn-server pull request #412: Fix integration tests in brooklyn-core
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/412#discussion_r86832920
--- Diff: core/src/test/java/org/apache/brooklyn/enricher/stock/EnrichersTest.java ---
@@ -201,17 +201,16 @@ public void testTransformingRespectsUnchangedButWillRepublish() {
entity.sensors().set(STR1, "myval");
Asserts.eventually(Suppliers.ofInstance(record), CollectionFunctionals.sizeEquals(1));
--- End diff --
As an aside, I noticed recently that `Asserts.eventually` has only a one second timeout, rather than the 30 second timeout of `Asserts.succeedsEventually` or `EntityAsserts.assertAttributeEqualsEventually`. I really don't like one second timeouts. They can fail in apache's jenkins if the VM/container is starved for a short length of time (admittedly I haven't seen that in a while, but I put that down to having changed lots of timeouts to use the `Asserts.DEFAULT_LONG_TIMEOUT`).
Anyway, that's not related to 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] brooklyn-server pull request #412: Fix integration tests in brooklyn-server
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/412#discussion_r87561101
--- Diff: software/base/src/test/java/org/apache/brooklyn/entity/brooklynnode/BrooklynNodeIntegrationTest.java ---
@@ -578,7 +578,7 @@ public void testUsesLocation() throws Exception {
assertEquals(parseJson(locatedLocationsContent, ImmutableList.of(appLocationId, "longitude"), Double.class), 45.6, 0.00001);
}
- @Test(groups="Integration")
+ @Test(groups={"Integration", "Broken"})
--- End diff --
What is causing all of these tests to fail? Any clues that could be given in a short comment?
I just tried running this one, and it passed locally for me (on OSX).
---
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] brooklyn-server pull request #412: Fix integration tests in brooklyn-core
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/412#discussion_r87355507
--- Diff: utils/common/src/test/java/org/apache/brooklyn/util/javalang/MemoryUsageTrackerTest.java ---
@@ -82,7 +82,7 @@ private long sizeOfActiveReferences(List<Maybe<byte[]>> references) {
return size;
}
- @Test(groups="Integration")
+ @Test(groups="Live")
--- End diff --
@ahgittin Do you agree with this change? The test is failing on [apache infra](https://builds.apache.org/view/Brooklyn/job/brooklyn-integration-tests/48/consoleFull), any idea why?
---
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] brooklyn-server issue #412: Fix integration tests in brooklyn-core
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/412
Jenkins test failure is:
```
testStartAndStopUnmanagedAppAutomanagesTheAppAndChildren(org.apache.brooklyn.core.entity.AbstractApplicationLegacyTest) Time elapsed: 2.243 sec <<< FAILURE!
java.lang.AssertionError: expected [Application[xhv6r1oi]] but found [null]
at org.testng.Assert.fail(Assert.java:94)
at org.testng.Assert.failNotEquals(Assert.java:513)
at org.testng.Assert.assertEqualsImpl(Assert.java:130)
at org.testng.Assert.assertEquals(Assert.java:116)
at org.testng.Assert.assertEquals(Assert.java:179)
at org.apache.brooklyn.core.entity.AbstractApplicationLegacyTest.testStartAndStopUnmanagedAppAutomanagesTheAppAndChildren(AbstractApplicationLegacyTest.java:95)
```
---
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] brooklyn-server pull request #412: Fix integration tests in brooklyn-server
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/412#discussion_r87563108
--- Diff: software/base/src/test/java/org/apache/brooklyn/entity/software/base/VanillaSoftwareProcessStreamsIntegrationTest.java ---
@@ -37,7 +37,10 @@ public void setUp() throws Exception {
localhost = app.getManagementContext().getLocationRegistry().getLocationManaged("localhost");
}
- @Test(groups = "Integration")
+ // Fails on subsequent runs because "BROOKLYN" marker already created in
--- End diff --
I'll do this in a new 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] brooklyn-server pull request #412: Fix integration tests in brooklyn-core
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/412#discussion_r87418101
--- Diff: utils/common/src/test/java/org/apache/brooklyn/util/javalang/MemoryUsageTrackerTest.java ---
@@ -82,7 +82,7 @@ private long sizeOfActiveReferences(List<Maybe<byte[]>> references) {
return size;
}
- @Test(groups="Integration")
+ @Test(groups="Live")
--- End diff --
I'm currently setting it up so it's still unofficial. It's running in a docker container which lets us set the environment as needed.
Note the messages says `cleared after 1000001` which means that the first reference never got cleared, it just got to the end of the loop. Looking at all references sounds good.
---
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] brooklyn-server issue #412: Fix integration tests in brooklyn-core
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/412
Changes look good - it certainly improves things. Happy for this to be merged, and then for us to fix the `testTransformingRespectsUnchangedButWillRepublish` later. Or for you to look at that as part of 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] brooklyn-server pull request #412: Fix integration tests in brooklyn-core
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/412#discussion_r87408752
--- Diff: utils/common/src/test/java/org/apache/brooklyn/util/javalang/MemoryUsageTrackerTest.java ---
@@ -82,7 +82,7 @@ private long sizeOfActiveReferences(List<Maybe<byte[]>> references) {
return size;
}
- @Test(groups="Integration")
+ @Test(groups="Live")
--- End diff --
I didn't realise that apache infra was running our integration tests. Thanks for the link!
No strong feelings from me. Or could mark is as group "Broken" until we get to the bottom of it. I'll play around in another PR at a possible fix. Will ping you when that's done!
Here's the output from apache infra's run. Note that the test took `1454694 ms`, compared to 3 seconds on my laptop (I run java 7). Note how many 1M objects we've created! I therefore believe the first (i.e. the one we check) was not being deleted, but lots of others were. We could check for any absent, rather than just the first.
```
2016-11-10 09:38:30,153 INFO First soft reference cleared after 1000001 1M blocks created; 999628 of them cleared
2016-11-10 09:38:30,158 INFO TESTNG FAILED: "Surefire test" - org.apache.brooklyn.util.javalang.MemoryUsageTrackerTest.testSoftUsageAndClearance() finished in 1454694 ms
java.lang.AssertionError: Should have had less than 10% free memory before clearance, had 266 MB / 716 MB expected [true] but found [false]
at org.apache.brooklyn.util.javalang.MemoryUsageTrackerTest.testSoftUsageAndClearance(MemoryUsageTrackerTest.java:104)
```
I wonder if it's related to @drigodwin's change in https://github.com/apache/brooklyn-server/blob/master/parent/pom.xml#L691 to add `-XX:SoftRefLRUPolicyMSPerMB=1` (as discussed in https://issues.apache.org/jira/browse/BROOKLYN-375). That could change the behaviour we see, for when and how much is collected. But I wouldn't have thought so, because that just changes the behaviour on collection (and shouldn't trigger a collection before one is really needed).
---
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] brooklyn-server issue #412: Fix integration tests in brooklyn-server
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/412
Great fixes and improvements, thanks @neykov - merging 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] brooklyn-server issue #412: Fix integration tests in brooklyn-core
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/412
Added some more improvements, see above commits.
---
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.
---