You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by nakomis <gi...@git.apache.org> on 2014/10/14 12:49:23 UTC
[GitHub] incubator-brooklyn pull request: Updates monit test to explicitly ...
GitHub user nakomis opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/241
Updates monit test to explicitly start process (vi) rather than assume '...
...cron' is running, uses 'rest of line' for status, rather than next word
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nakomis/incubator-brooklyn fix/monit-integration-test
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/241.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 #241
----
commit d46d8ccb2833ca1b502107d8cfa39e4b6d114093
Author: Martin Harris <gi...@nakomis.com>
Date: 2014-10-14T10:46:07Z
Updates monit test to explicitly start process (vi) rather than assume 'cron' is running, uses 'rest of line' for status, rather than next word
----
---
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: Updates monit test to explicitly ...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/241#issuecomment-59188047
@nakomis can you close and re-open this PR to trigger apache jenkins again 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: Updates monit test to explicitly ...
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/241#discussion_r18886318
--- Diff: software/monitoring/src/test/java/brooklyn/entity/monitoring/monit/MonitIntegrationTest.java ---
@@ -49,37 +50,26 @@
import com.google.common.io.Files;
public class MonitIntegrationTest extends BrooklynAppLiveTestSupport {
-
- /*
- * FIXME Has the monit node output changed? Now it is listing 'cron' as well as system.
- * It keeps reporting "Does" rather than "Running" as the status.
- * Should we be getting the rest of the line after "status" (and trimming), rather than
- * just the first word?
-
- input=The Monit daemon 5.9 uptime: 0m
- Process 'cron'
- status Does not exist
- monitoring status Monitored
- data collected Mon, 13 Oct 2014 21:27:01
- System 'aleds-macbook-pro.local'
- status Running
- monitoring status Monitored
- load average [5.16] [4.73] [4.45]
- cpu 7.1%us 5.7%sy
- memory usage 11.6 GB [72.3%]
- swap usage 3.0 GB [74.7%]
- data collected Mon, 13 Oct 2014 21:27:01
- */
-
private static final Logger LOG = LoggerFactory.getLogger(MonitIntegrationTest.class);
LocalhostMachineProvisioningLocation loc;
+ Process testProcess;
@BeforeMethod(alwaysRun=true)
public void setUp() throws Exception {
super.setUp();
loc = app.newLocalhostProvisioningLocation();
+ testProcess = (new ProcessBuilder()).command("vi", "monittest").start();
--- End diff --
Personal preference would be to have used a filename that included a random id, to ensure that tests can never interfere with each other. But this should be ok, as our tests are not run in parallel and tearDown looks ok (as long as `super.tearDown` doesn't throw an exception, given it's not in a try-finally block!)
---
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: Updates monit test to explicitly ...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/241#issuecomment-59188891
Looks good. Just the one minor comment about not trimming getRemainderOfLineAfter.
---
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: Updates monit test to explicitly ...
Posted by nakomis <gi...@git.apache.org>.
Github user nakomis commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/241#issuecomment-59201031
Tests should now pass
---
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: Updates monit test to explicitly ...
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/241#discussion_r18886476
--- Diff: utils/common/src/main/java/brooklyn/util/text/Strings.java ---
@@ -537,6 +537,18 @@ public static String getFirstWordAfter(String context, String phrase) {
return getFirstWord(context.substring(index + phrase.length()));
}
+ public static String getRemainderOfLineAfter(String context, String phrase) {
+ if (context == null || phrase == null) return null;
+ int index = context.indexOf(phrase);
+ if (index < 0) return null;
+ int lineEndIndex = context.indexOf("\n", index);
+ if (lineEndIndex <= 0) {
+ return trim(context.substring(index + phrase.length()));
--- End diff --
I don't think this should trim the result. That's not what I'd expect from the wording `getRemainderOfLineAfter`. Let the caller trim instead, if it wants to.
---
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: Updates monit test to explicitly ...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/241#issuecomment-59197679
jenkins test failure looks related:
java.lang.AssertionError: expected [hello] but found [ hello]
at org.testng.Assert.fail(Assert.java:94)
at org.testng.Assert.failNotEquals(Assert.java:494)
at org.testng.Assert.assertEquals(Assert.java:123)
at org.testng.Assert.assertEquals(Assert.java:176)
at org.testng.Assert.assertEquals(Assert.java:186)
at brooklyn.util.text.StringsTest.testGetRemainderOfLineAfter(StringsTest.java:345)
---
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: Updates monit test to explicitly ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/241
---
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: Updates monit test to explicitly ...
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/241#discussion_r18886557
--- Diff: software/monitoring/src/test/java/brooklyn/entity/monitoring/monit/MonitIntegrationTest.java ---
@@ -49,37 +50,26 @@
import com.google.common.io.Files;
public class MonitIntegrationTest extends BrooklynAppLiveTestSupport {
-
- /*
- * FIXME Has the monit node output changed? Now it is listing 'cron' as well as system.
- * It keeps reporting "Does" rather than "Running" as the status.
- * Should we be getting the rest of the line after "status" (and trimming), rather than
- * just the first word?
-
- input=The Monit daemon 5.9 uptime: 0m
- Process 'cron'
- status Does not exist
- monitoring status Monitored
- data collected Mon, 13 Oct 2014 21:27:01
- System 'aleds-macbook-pro.local'
- status Running
- monitoring status Monitored
- load average [5.16] [4.73] [4.45]
- cpu 7.1%us 5.7%sy
- memory usage 11.6 GB [72.3%]
- swap usage 3.0 GB [74.7%]
- data collected Mon, 13 Oct 2014 21:27:01
- */
-
private static final Logger LOG = LoggerFactory.getLogger(MonitIntegrationTest.class);
LocalhostMachineProvisioningLocation loc;
+ Process testProcess;
@BeforeMethod(alwaysRun=true)
public void setUp() throws Exception {
super.setUp();
loc = app.newLocalhostProvisioningLocation();
+ testProcess = (new ProcessBuilder()).command("vi", "monittest").start();
--- End diff --
Actually, ignore me: this needs to match the contents of `monit.monitrc`.
---
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: Updates monit test to explicitly ...
Posted by nakomis <gi...@git.apache.org>.
Github user nakomis commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/241#issuecomment-59196470
PR comments addressed
---
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.
---