2

I'm going through our application's unit tests and improving/adding more of them. I'm quite (no, very) novice in unit testing/test-driven development and I found the following method that I wanted to test. I'm stuck, and my question is if there's a way to rewrite this so that it is testable?

public static bool Is32BitOS()
{
        string os = (from x in new ManagementObjectSearcher("SELECT * FROM Win32_OperatingSystem").Get().OfType<ManagementObject>()
                     select x.GetPropertyValue("Caption")).First().ToString().Trim();

        if (os.Equals("Microsoft Windows XP Professional"))
        {
            return true;
        }

        if (os.StartsWith("Microsoft Windows 7"))
        {
            string architecture = (from x in new ManagementObjectSearcher("SELECT * FROM Win32_OperatingSystem").Get().OfType<ManagementObject>()
                                   select x.GetPropertyValue("OSArchitecture")).First().ToString();
            if (architecture == "64-bit")
            {
                return false;
            }
        }

        return true;
}
Mario S
  • 11,715
  • 24
  • 39
  • 47
Amadeus Hein
  • 706
  • 1
  • 4
  • 12
  • 2
    You can always wrap the `ManagementObjectSearcher` in a thin interface, which will automatically make your dependency clearly visible and easily swappable for other implementation, like the test one... Just an idea (and I'm on the phone, so it's brief ;)) – Patryk Ćwiek Apr 18 '13 at 13:56
  • 2
    If you're using .NET 4.0, take a look at `Environment.Is64BitOperatingSystem()` method http://msdn.microsoft.com/en-us/library/system.environment.is64bitoperatingsystem%28VS.100%29.aspx – bniwredyc Apr 18 '13 at 13:57
  • Check [this question](http://stackoverflow.com/q/336633/706456) – oleksii Apr 18 '13 at 14:28
  • 1
    @bniwredyc It was a nice feeling to bring that function to the senior devs! The best unit test must surely be one I don't need to write! – Amadeus Hein Apr 19 '13 at 06:24
  • @Trustme-I'maDoctor You're right, a good way to unit test is to interface these messy things. Luckily, we use .NET 4.0, but I will think about this for future tests! Thanks a lot! :-) – Amadeus Hein Apr 19 '13 at 06:26

1 Answers1

2

Your method has 3 responsibilities:

  1. management object searcher creation
  2. os and architecture retrieval
  3. os verification

To enable testing I'd add in AssemblyInfo.cs a line similar to this:

[assembly: InternalsVisibleTo("Your.Test.Project")]

so that the protected methods will be visible to the test project but not freely available to clients. Then I'd rewrite your method so that the os verification is separated:

//Acceptance test this method
public static bool Is32BitOS()
{
    var managementObjects = new ManagementObjectSearcher("SELECT * FROM Win32_OperatingSystem").Get().OfType<ManagementObject>();

    string os = (from x in managementObjects select x.GetPropertyValue("Caption")).First().ToString().Trim();
    string architecture = (from x in managementObjects select x.GetPropertyValue("OSArchitecture")).First().ToString();

    return Is32BitOS(os, architecture);
}

//Unit test this method
internal static bool Is32BitOS(string os, string architecture)
{
    if (os.Equals("Microsoft Windows XP Professional"))
    {
        return true;
    }

    if (os.StartsWith("Microsoft Windows 7"))
    {
        string architecture = archRetriever();
        if (architecture == "64-bit")
        {
            return false;
        }
    }
    return true;
}

Now that we have separated concerns I'd say that your Unit Test should only verify the Is32BitOs behaviour while an Acceptance Test should verify the end to end stack. In fact you have little value in unit testing that

(from x in new ManagementObjectSearcher("SELECT * FROM Win32_OperatingSystem").Get().OfType<ManagementObject>()
 select x.GetPropertyValue("Caption")).First().ToString().Trim();

retrieves the os string while your real value resides in the usage of this info to determine if the Os is 32 bit.

An alternative to wrap things up in interfaces and use mocking frameworks is to apply functional programming look here for Llewellyn Falco's peel and slice technique and here Arlo Belshee's no mocks approach. So instead of a method like:

public static bool Is32BitOS(IRetrieveOsAndArchitecture roa)
{   
    string os = roa.GetOs();                
    string architecture = roa.GetArchitecure();

    return Is32BitOS(os, architecture);
}

You could use something like:

public static bool Is32BitOS(Func<ManagementObjectSearcher, string> osRetriever, 
                             Func<ManagementObjectSearcher, string> archRetriever, 
                             ManagementObjectSearcher searcher)
{   
    string os = osRetriever(searcher);              
    string architecture = archRetriever(searcher);

    return Is32BitOS(os, architecture);
}

Its client method will be:

public static bool Is32BitOS()
{
    var searcher = new ManagementObjectSearcher("SELECT * FROM Win32_OperatingSystem");

    return Is32Bit((s)=>{ (from x in s.Get().OfType<ManagementObject>() select x.GetPropertyValue("Caption")).First().ToString().Trim()},
                   (s)=>{ (from x in s.Get().OfType<ManagementObject>() select x.GetPropertyValue("OSArchitecture")).First().ToString();},
                   searcher);
}

Notice that while testing the cases of interfaces and functional you are not using the real ManagementObjectSearcher external dependency; in mockist approach you'll use a mock object in functional programming you'll pass in a different lambda expression which should return only the os and architecture strings.

There's still a responsibility left to this method and is the creation of the ManagementObjectSearcher; to resolve this dependency you can:

  1. pass it as the parameter of the method
  2. make it a field of your class initilized at construction time