Fix integration tests in brooklyn-server #412

neykov · 2016-11-07T17:13:28Z

No description provided.

aledsage · 2016-11-07T18:09:09Z

core/src/test/java/org/apache/brooklyn/enricher/stock/EnrichersTest.java

        Asserts.assertThat(record.getEvents(), CollectionFunctionals.sizeEquals(0));

        entity.sensors().set(STR1, "myval");
        Asserts.eventually(Suppliers.ofInstance(record), CollectionFunctionals.sizeEquals(1));


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.

aledsage · 2016-11-07T18:18:35Z

core/src/test/java/org/apache/brooklyn/enricher/stock/EnrichersTest.java

        entity.sensors().set(STR1, "myval");
        Asserts.eventually(Suppliers.ofInstance(record), CollectionFunctionals.sizeEquals(1));
-        EntityAsserts.assertAttributeEquals(entity, STR2, "myval");
+        EntityAsserts.assertAttributeEqualsEventually(entity, STR2, "myval");


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)

i.e. we include null as the first event notification.

aledsage · 2016-11-07T18:20:59Z

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.

neykov · 2016-11-08T13:06:45Z

@aledsage Addressed comments plus some additional changes.
Agree we should increase the eventually timeout.

neykov · 2016-11-10T08:55:52Z

Added some more improvements, see above commits.

neykov · 2016-11-10T09:23:06Z

utils/common/src/test/java/org/apache/brooklyn/util/javalang/MemoryUsageTrackerTest.java

    }

-    @Test(groups="Integration")
+    @Test(groups="Live")


@ahgittin Do you agree with this change? The test is failing on apache infra, any idea why?

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).

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.

See #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.

aledsage · 2016-11-10T14:35:42Z

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)

@neykov

Improve robustness of MemoryUsageTrackerTest Motivated by discussion in #412 with @neykov (cc @ahgittin).

Now that we don't support Java 6 there's no restriction on upgrading testng. The new version could improve the execution order by not interleaving tests from different classes. But seems that's not the case when using dependsOn*

brooklyn-software-base, brooklyn-camp, brooklyn-rest-resources

aledsage · 2016-11-11T09:38:50Z

core/src/test/java/org/apache/brooklyn/core/entity/AbstractApplicationLegacyTest.java

        assertTrue(Entities.isManaged(app2));
        assertTrue(Entities.isManaged(child));
        assertEquals(child.getCallHistory(), ImmutableList.of("start"));
-        assertEquals(mgmt.getEntityManager().getEntity(app2.getId()), app2);


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!

aledsage · 2016-11-11T09:41:45Z

.../base/src/test/java/org/apache/brooklyn/entity/brooklynnode/BrooklynNodeIntegrationTest.java

    }

-    @Test(groups="Integration")
+    @Test(groups={"Integration", "Broken"})


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).

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.

aledsage · 2016-11-11T09:49:03Z

...a/org/apache/brooklyn/entity/software/base/VanillaSoftwareProcessStreamsIntegrationTest.java

    }

-    @Test(groups = "Integration")
+    // Fails on subsequent runs because "BROOKLYN" marker already created in 


This can also be fixed by adding:

.configure(VanillaSoftwareProcess.INSTALL_UNIQUE_LABEL, Identifiers.makeRandomId(8))

I'll do this in a new PR.

aledsage · 2016-11-11T09:49:41Z

Great fixes and improvements, thanks @neykov - merging now.

aledsage reviewed Nov 7, 2016

View reviewed changes

neykov force-pushed the fix/integration-tests-core branch 2 times, most recently from 640847d to d76846b Compare November 8, 2016 12:42

neykov force-pushed the fix/integration-tests-core branch from 450ed6e to fef4750 Compare November 9, 2016 16:15

neykov force-pushed the fix/integration-tests-core branch 2 times, most recently from 3d0a922 to a05ffc7 Compare November 10, 2016 09:11

neykov commented Nov 10, 2016

View reviewed changes

aledsage mentioned this pull request Nov 10, 2016

Improve robustness of MemoryUsageTrackerTest #426

Merged

neykov force-pushed the fix/integration-tests-core branch from 46536fc to 103809f Compare November 10, 2016 15:43

asfgit pushed a commit that referenced this pull request Nov 10, 2016

Closes #426

70f2950

Improve robustness of MemoryUsageTrackerTest Motivated by discussion in #412 with @neykov (cc @ahgittin).

neykov added 5 commits November 10, 2016 18:06

Fix integration tests in brooklyn-core

b714516

Convert integration tests to unit tests and fix timing related failures

89646d5

Use ssh cli in batch mode - don't ask for passwords

69c91c3

Upgrade testng to latest version

2d3caac

Now that we don't support Java 6 there's no restriction on upgrading testng. The new version could improve the execution order by not interleaving tests from different classes. But seems that's not the case when using dependsOn*

brooklyn-locations-jclouds integration test fixes

b6af270

neykov force-pushed the fix/integration-tests-core branch from 103809f to b6af270 Compare November 10, 2016 16:07

neykov changed the title ~~Fix integration tests in brooklyn-core~~ Fix integration tests in brooklyn-server Nov 10, 2016

neykov force-pushed the fix/integration-tests-core branch 3 times, most recently from a1f2e7d to ae16722 Compare November 10, 2016 20:42

Make sure entity doesn't equal null

fe2b4b6

neykov force-pushed the fix/integration-tests-core branch from ae16722 to a3a0544 Compare November 10, 2016 20:47

more integration test fixes

3ec0dd4

brooklyn-software-base, brooklyn-camp, brooklyn-rest-resources

neykov force-pushed the fix/integration-tests-core branch from a3a0544 to 3ec0dd4 Compare November 10, 2016 21:04

aledsage reviewed Nov 11, 2016

View reviewed changes

asfgit merged commit 3ec0dd4 into apache:master Nov 11, 2016

asfgit pushed a commit that referenced this pull request Nov 11, 2016

This closes #412

1caa48f

neykov deleted the fix/integration-tests-core branch November 11, 2016 10:01

Fix integration tests in brooklyn-server #412

Fix integration tests in brooklyn-server #412

Uh oh!

Conversation

neykov commented Nov 7, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aledsage commented Nov 7, 2016

Uh oh!

neykov commented Nov 8, 2016

Uh oh!

neykov commented Nov 10, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aledsage commented Nov 10, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aledsage commented Nov 11, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants