brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ahgittin <...@git.apache.org>
Subject [GitHub] incubator-brooklyn pull request: Updates to Test framework asserti...
Date Wed, 25 Nov 2015 09:14:09 GMT
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1066#discussion_r45843346
  
    --- Diff: usage/test-framework/src/main/java/org/apache/brooklyn/test/framework/TestHttpCallImpl.java
---
    @@ -66,31 +68,35 @@ public void start(Collection<? extends Location> locations)
{
             }
         }
     
    -    private Supplier<String> buildDataSupplier(final HttpAssertionTarget httpAssertionTarget,
final String url) {
    -
    -        switch (httpAssertionTarget) {
    +    private void doRequestAndCheckAssertions(Map<String, Duration> flags, List<Map<String,
Object>> assertions,
    +                                             HttpAssertionTarget target, final String
url) {
    +        switch (target) {
                 case body:
    -                return new Supplier<String>() {
    +                Supplier<String> getBody = new Supplier<String>() {
                         @Override
                         public String get() {
                             return HttpTool.getContent(url);
                         }
                     };
    +                TestFrameworkAssertions.checkAssertions(flags, assertions, target.toString(),
getBody);
    +                break;
                 case status:
    -                return new Supplier<String>() {
    +                Supplier<Integer> getStatusCode = new Supplier<Integer>()
{
                         @Override
    -                    public String get() {
    +                    public Integer get() {
                             try {
    -                            return String.valueOf(HttpTool.getHttpStatusCode(url));
    +                            return HttpTool.getHttpStatusCode(url);
                             } catch (Exception e) {
    -                            LOG.error("HTTP call to [{}] failed due to [{}] ... returning
Status code [0 - Unreachable]", url, e.getMessage());
    -                            return "0";
    +                            LOG.error("HTTP call to [{}] failed due to [{}] ... returning
Status code [500]",
    --- End diff --
    
    definitely not ERROR level, since here we're just in a status check.
    
    normally in catch blocks where we swallow generic exceptions we include
    
            Exceptions.propagateIfFatal(e);
    
    in case it's a severe exception.  but here' i'd simply propagate it regardless (`Exceptions.propagate(e)')
and make sure the wrapper code treats errors appropriately.  returning `500` is dishonest
:) .


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

Mime
View raw message