cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jburwell <...@git.apache.org>
Subject [GitHub] cloudstack pull request: Fix Sync of template.properties in Swift
Date Fri, 22 Apr 2016 13:56:35 GMT
Github user jburwell commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1331#discussion_r60742145
  
    --- Diff: services/secondary-storage/server/test/org/apache/cloudstack/storage/resource/TestAppender.java
---
    @@ -0,0 +1,173 @@
    +/*
    +* 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.cloudstack.storage.resource;
    +
    +import com.google.common.base.Joiner;
    +import com.google.common.base.Objects;
    +import com.google.common.collect.ImmutableMap;
    +import org.apache.log4j.AppenderSkeleton;
    +import org.apache.log4j.Level;
    +import org.apache.log4j.Logger;
    +import org.apache.log4j.spi.LoggingEvent;
    +import java.util.ArrayList;
    +import java.util.HashMap;
    +import java.util.HashSet;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +import java.util.regex.Pattern;
    +import static com.google.common.base.Preconditions.checkArgument;
    +import static com.google.common.base.Preconditions.checkState;
    +import static com.google.common.base.Strings.isNullOrEmpty;
    +import static java.lang.String.format;
    +import static org.apache.log4j.Level.ALL;
    +import static org.apache.log4j.Level.DEBUG;
    +import static org.apache.log4j.Level.ERROR;
    +import static org.apache.log4j.Level.FATAL;
    +import static org.apache.log4j.Level.INFO;
    +import static org.apache.log4j.Level.OFF;
    +import static org.junit.Assert.fail;
    +
    +/**
    +*
    +* Tracks one or more patterns to determine whether or not they have been
    +* logged. It uses a streaming approach to determine whether or not a message
    +* has a occurred to prevent unnecessary memory consumption. Instances of this
    +* of this class are created using the {@link TestAppenderBuilder}.
    +*
    +* To use this class, register a one or more expected patterns by level as part
    +* of the test setup and retain an reference to the appender instance. After the
    +* expected logging events have occurred in the test case, call
    +* {@link TestAppender#assertMessagesLogged()} which will fail the test if any of the
    +* expected patterns were not logged.
    +*
    +*/
    +public final class TestAppender extends AppenderSkeleton {
    +    private final static String APPENDER_NAME = "test_appender";
    +    private final ImmutableMap<Level, Set<PatternResult>> expectedPatternResults;
    +    private TestAppender(final Map<Level, Set<PatternResult>> expectedPatterns)
{
    +        super();
    +        expectedPatternResults = ImmutableMap.copyOf(expectedPatterns);
    +    }
    +    protected void append(LoggingEvent loggingEvent) {
    +        checkArgument(loggingEvent != null, "append requires a non-null loggingEvent");
    +        final Level level = loggingEvent.getLevel();
    +        checkState(expectedPatternResults.containsKey(level), "level " + level + " not
supported by append");
    +        for (final PatternResult patternResult : expectedPatternResults.get(level)) {
    +            if (patternResult.getPattern().matcher(loggingEvent.getRenderedMessage()).matches())
{
    +                patternResult.markFound();
    +            }
    +        }
    +    }
    +    public void close() {
    +// Do nothing ...
    +    }
    +    public boolean requiresLayout() {
    +        return false;
    +    }
    +    public void assertMessagesLogged() {
    +        final List<String> unloggedPatterns = new ArrayList<>();
    +        for (final Map.Entry<Level, Set<PatternResult>> expectedPatternResult
: expectedPatternResults.entrySet()) {
    +            for (final PatternResult patternResults : expectedPatternResult.getValue())
{
    +                if (!patternResults.isFound()) {
    +                    unloggedPatterns.add(format("%1$s was not logged for level %2$s",
    +                            patternResults.getPattern().toString(), expectedPatternResult.getKey()));
    +                }
    +            }
    +        }
    +        if (!unloggedPatterns.isEmpty()) {
    +            fail(Joiner.on(",").join(unloggedPatterns));
    +        }
    +    }
    +    private static final class PatternResult {
    +        private final Pattern pattern;
    +        private boolean foundFlag = false;
    +        private PatternResult(Pattern pattern) {
    +            super();
    +            this.pattern = pattern;
    +        }
    +        public Pattern getPattern() {
    +            return pattern;
    +        }
    +        public void markFound() {
    +        // This operation is thread-safe because the value will only ever be switched
from false to true. Therefore,
    +        // multiple threads mutating the value for a pattern will not corrupt the value
...
    +            foundFlag = true;
    +        }
    +        public boolean isFound() {
    +            return foundFlag;
    +        }
    +        @Override
    +        public boolean equals(Object thatObject) {
    +            if (this == thatObject) {
    +                return true;
    +            }
    +            if (thatObject == null || getClass() != thatObject.getClass()) {
    +                return false;
    +            }
    +            PatternResult thatPatternResult = (PatternResult) thatObject;
    +            return foundFlag == thatPatternResult.foundFlag &&
    +                    Objects.equal(pattern, thatPatternResult.pattern);
    +        }
    +        @Override
    +        public int hashCode() {
    +            return Objects.hashCode(pattern, foundFlag);
    +        }
    +        @Override
    +        public String toString() {
    +            return format("Pattern Result [ pattern: %1$s, markFound: %2$s ]", pattern.toString(),
foundFlag);
    +        }
    +    }
    +
    +    public static final class TestAppenderBuilder {
    --- End diff --
    
    Minor Nit: I should have named this class ``Builder`` since it is already namespaced ``TestAppender``.
  I would recommend that change since ``new TestAppender.Builder()`` is cleaner than ``newTestAppender.TestAppenderBuilder()``.
 (Cosmetic I know, but it hits my naming OCD)


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