You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ahgittin <gi...@git.apache.org> on 2016/11/07 11:29:43 UTC
[GitHub] brooklyn-server pull request #411: Test change to coercion of default config...
GitHub user ahgittin opened a pull request:
https://github.com/apache/brooklyn-server/pull/411
Test change to coercion of default config values
Several minor fixes/changes, plus one significant test in https://github.com/apache/brooklyn-server/commit/e9c1f4cbb67d1bd89d1e7d84f8730225047ea337 which highlights a change in behaviour since early Oct
Also see related PR adding this to release notes
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ahgittin/brooklyn-server default-conversion-notes
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/411.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 #411
----
commit dc52db3fc6030dc8e863d6c71794ac7d0bcf732b
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2016-11-07T11:16:56Z
more conveniences for Strings
commit ef3942166b031b10157bc65708fb533eba4ad60c
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2016-11-07T11:17:15Z
let yaml tests look in the package where the class is defined
(so we can group things better than all in the root)
commit e9c1f4cbb67d1bd89d1e7d84f8730225047ea337
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2016-11-07T11:21:41Z
new test showing changed behaviour of default config value resolution
as included in the release notes
commit c70959099cbd8552dca5b36f573a293a1cd04e1f
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2016-11-07T11:22:14Z
tidy warnings (deprecation, unused) for camp tests
----
---
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 #411: Test change to coercion of default config...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/411#discussion_r86764782
--- Diff: camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java ---
@@ -112,7 +113,14 @@ protected void waitForApplicationTasks(Entity app) {
}
protected String loadYaml(String yamlFileName, String ...extraLines) throws Exception {
- String input = new ResourceUtils(this).getResourceAsString(yamlFileName).trim();
+ ResourceUtils ru = new ResourceUtils(this);
+ if (!ru.doesUrlExist(yamlFileName)) {
+ if (ru.doesUrlExist(Urls.mergePaths(getClass().getPackage().getName().replace('.', '/'), yamlFileName))) {
+ // look in package-specific folder if not found at root
+ yamlFileName = Urls.mergePaths(getClass().getPackage().getName().replace('.', '/'), yamlFileName);
--- End diff --
the package-specific name is only used if the original path **isn't** found *and* the package-specific one **is** -- in which case i think it's correct to use the package-specific name in the error
---
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 #411: Test change to coercion of default config values
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/411
@ahgittin LGTM; only one very minor comment for you to take a look at.
---
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 #411: Test change to coercion of default config values
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/411
jenkins confirmed this passes - 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 pull request #411: Test change to coercion of default config...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/411
---
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 #411: Test change to coercion of default config...
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/411#discussion_r86761383
--- Diff: camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java ---
@@ -112,7 +113,14 @@ protected void waitForApplicationTasks(Entity app) {
}
protected String loadYaml(String yamlFileName, String ...extraLines) throws Exception {
- String input = new ResourceUtils(this).getResourceAsString(yamlFileName).trim();
+ ResourceUtils ru = new ResourceUtils(this);
+ if (!ru.doesUrlExist(yamlFileName)) {
+ if (ru.doesUrlExist(Urls.mergePaths(getClass().getPackage().getName().replace('.', '/'), yamlFileName))) {
+ // look in package-specific folder if not found at root
+ yamlFileName = Urls.mergePaths(getClass().getPackage().getName().replace('.', '/'), yamlFileName);
--- End diff --
Very minor (feel free to ignore). If it still can't be found, I'd prefer the final exception to give the original `yamlFileName` rather than the package-specific name. With this change, it will throw the exception in `run.getResourceAsString` with the transformed value.
---
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 #411: Test change to coercion of default config...
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/411#discussion_r86766218
--- Diff: camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java ---
@@ -112,7 +113,14 @@ protected void waitForApplicationTasks(Entity app) {
}
protected String loadYaml(String yamlFileName, String ...extraLines) throws Exception {
- String input = new ResourceUtils(this).getResourceAsString(yamlFileName).trim();
+ ResourceUtils ru = new ResourceUtils(this);
+ if (!ru.doesUrlExist(yamlFileName)) {
+ if (ru.doesUrlExist(Urls.mergePaths(getClass().getPackage().getName().replace('.', '/'), yamlFileName))) {
+ // look in package-specific folder if not found at root
+ yamlFileName = Urls.mergePaths(getClass().getPackage().getName().replace('.', '/'), yamlFileName);
--- End diff --
Ah, you're absolutely right - I missed that second `ru.doesUrlExist` check.
---
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 #411: Test change to coercion of default config...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/411#discussion_r86764801
--- Diff: camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigTypeCoercionYamlTest.java ---
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.camp.brooklyn;
+
+import java.util.Map;
+
+import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.test.Asserts;
+import org.apache.brooklyn.util.core.internal.ssh.RecordingSshTool;
+import org.apache.brooklyn.util.text.StringPredicates;
+import org.apache.brooklyn.util.text.Strings;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+public class ConfigTypeCoercionYamlTest extends AbstractYamlTest {
+ private static final Logger log = LoggerFactory.getLogger(ConfigTypeCoercionYamlTest.class);
+
+ @BeforeMethod(alwaysRun=true)
+ @Override
+ public void setUp() throws Exception {
+ super.setUp();
+ RecordingSshTool.clear();
+ }
+
+ @Test
+ public void testSshConfigFromDefault() throws Exception {
+ RecordingSshTool.setCustomResponse(".*myCommand.*", new RecordingSshTool.CustomResponse(0, "myResponse", null));
+
+ String bp = loadYaml("config-type-coercion-test.yaml",
+ "location:",
+ " localhost:",
+ " sshToolClass: "+RecordingSshTool.class.getName());
+ // remove all lines referring to "exact" -- that's useful for expository and running in UI
+ // but it will fail (timeout) if the port isn't available so not good in tests
+ bp = Strings.removeLines(bp, StringPredicates.containsLiteralIgnoreCase("exact"));
--- End diff --
agree
---
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 #411: Test change to coercion of default config values
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/411
thanks @aledsage, fixed, plus RAT fix, retesting then will 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.
---