2
\$\begingroup\$

The class helps to determine the exact output log message, taking in account the conversionPattern that log4net uses for layout. Current usage is you have to attach the listener, then call your BL or a wrapper method which your code uses to write to log, then call the GetRenderedMessage() method. The purpose is to make sure that even if I modify my wrapper method (I have a wrapper method which adds some properties to log4net's ThreadContext.Properties and also formats the message in a certain way) the rendered log would remain the same. Are there any improvements or suggestions to make it more "friendly" so someone would know how to use it without reading this intro?

using log4net;
using log4net.Appender;
using log4net.Config;
using System;
using System.IO;
using System.Linq;

namespace TestUtils
{
    public class Log4netListener
    {
        MemoryAppender memAppender;

        /// <summary>
        /// Attaches the listener so it would capture any log messages written.
        /// </summary>
        /// <param name="appenderName"></param>
        public void AttachLogListener(string appenderName)
        {
            var appender = LogManager.GetRepository().GetAppenders().Where(a => a.Name == appenderName);
            var patternLayout = ((log4net.Layout.PatternLayout)((AppenderSkeleton)(appender.First())).Layout);
            memAppender = new MemoryAppender();
            memAppender.Layout = patternLayout;
            BasicConfigurator.Configure(memAppender);
        }

        /// <summary>
        /// Gets the message written to log. Use this method only after AttachLogListener() was called
        /// and a message was written to log4net afterwards.
        /// </summary>       
        public string GetRenderedMessage()
        {
            if (memAppender == null)
            {
                throw new InvalidOperationException("The listener was not attached. Make sure you call AttachLogListener() before writing to log.");
            }
            var events = memAppender.GetEvents();
            string logMessage = "";
            using (StringWriter writer = new StringWriter())
            {
                foreach (var ev in events)
                {
                    memAppender.Layout.Format(writer, ev);
                }
                logMessage = writer.ToString();
            }
            return logMessage;
        }
    }
}

Usage:

[TestClass]
public class LoggingTests
{
    [AssemblyInitialize]
    public static void Configure(TestContext tc)
    {
        XmlConfigurator.Configure(); // must use this line to make log4net work from test  if "configSource" is used in app.config
    }

    [TestMethod]
    public void LogLayout()
    {
        var listener = new Log4netListener();
        listener.AttachLogListener("MyLogFileAppender");

        // Call business logic here that writes to log...

        string logMessage = listener.GetRenderedMessage();
        // Assert...
    }
}
\$\endgroup\$
1
  • \$\begingroup\$ Using constructor to attach an appender instead of the AttachLogListener method is not an option? \$\endgroup\$
    – Maxim
    Jul 24, 2017 at 13:22

2 Answers 2

1
\$\begingroup\$

Are there any improvements or suggestions to make it more "friendly" so someone would know how to use it without reading this intro?

The most obvious thing is high-quality XML docs. You've already started, but you need to fill in the param name where it's blank, and the return value for the method where it's missing.

On top of that, adding a summary tag to the class itself containing most of the verbiage above would be helpful.

Reading a little further into your implementation - is it possible to move the code of the AttachLogListener method into a constructor? That would guarantee your order of execution/initialization and make usage more sane and obvious, I think.

\$\endgroup\$
2
  • \$\begingroup\$ Thank you for the enlightenments, regarding the constructor - my concern was that if I put the logic in the constructor then someone would might try to write logger.Error("bla"); var listener = new Log4netListener("myAppender"); listener.GetRenderedMessage() but it won't work because the "attach" must be done before calling logger.Error("bla"); so I thought that adding an "Attach" method would convey this message (that you have to call Attach before actual logging) more clearly. Your opinion? \$\endgroup\$
    – BornToCode
    Jul 25, 2017 at 7:22
  • \$\begingroup\$ @BornToCode IMO, programmatic enforcement is always the better option. As long as your code is adequately documented, it's better to make one wrong thing impossible (construct-without-attach) than to make zero wrong things impossible. \$\endgroup\$
    – Reinderien
    Jul 25, 2017 at 16:14
0
\$\begingroup\$

I used your class, and added 2 methods to simplify usage in my unit tests:

    public void AttachLogListener()
    {
        memAppender = new MemoryAppender();
        BasicConfigurator.Configure(memAppender);
    }

   public string GetMessages()
    {
        if (memAppender == null)
        {
            throw new InvalidOperationException("The listener was not attached. Make sure you call AttachLogListener() before writing to log.");
        }
        var events = memAppender.GetEvents();
        string logMessage;
        using (var writer = new StringWriter())
        {
            foreach (var ev in events)
            {
                ev.WriteRenderedMessage(writer);
            }
            logMessage = writer.ToString();
        }
        return logMessage;
    }

In my test, I didn't care anything about the message format, just wanted to ensure that the expected error was logged, and contained certain pieces of text. So my unit test looked like this:

    [Test]
    public void TestOnConnectErrorLogs()
    {
        var listener = new Log4NetListener();
        listener.SetupAppender();

        const string errMsg = "Failed to connect!!";
        client.onConnectError(errMsg);

        var msg = listener.GetMessages();
        Assert.IsNotNull(msg, "found logged message");
        Assert.IsTrue(msg.Contains(errMsg), "logged root error. expected={0}, was={1}", errMsg, msg);
    }

Some additional important points:

  1. There needs to be a check for empty list in the AttachLogListener method. If the programmer uses a bad appender name, he gets System.InvalidOperationException : Sequence contains no elements error. It is better to check that condition that should check and throw Exception with a helpful message.

  2. Because the Repository is static, every time this class is used, it will create and add a new MemoryAppender. This is kind of a memory leak. There should be a clean up method to remove the Appender when done.

Example code to address the above 2 points:

public class Log4NetListener : IDisposable
{   
    // Handy reference to Log4Net Root Logger
    private static Logger Root
    {
        get { return ((Hierarchy)LogManager.GetRepository()).Root; }
    }

    /// <summary>
    /// Attaches the listener so it would capture any log messages written.
    /// </summary>
    /// <param name="appenderName">Name of Appender which has a PatternLayout to use for writing test log messages</param>
    /// <exception cref="InvalidOperationException">if no Appender exists with specified name</exception>
    public void AttachLogListener(string appenderName)
    {
        var appender = LogManager.GetRepository().GetAppenders().Where(a => a.Name == appenderName).ToList();
        if (appender.Count == 0)
        {
            throw new InvalidOperationException(string.Format("Unable to find Appender named [{0}]! unable to setup Appender...", appenderName));
        }
        var patternLayout = ((PatternLayout)((AppenderSkeleton)(appender.First())).Layout);
        CreateAppender(patternLayout);

    }

Also, as someone suggested above, handling the setup in constructor, or else perhaps adding a static method to make setup (and teardown) one-liners would be nice.

There is more that could be done to make this class useful. I have a similar utility class in Java, and added helper methods to select by Level, or using regexp matching. (These are helpful where a test could generate many log messages, and you just want to pick out a few.)

The version of this that I ended up using can be downloaded from here.

In summary, I think this was very good starting point, and easy to use.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.