6

I had visual studio create a test for each member in my class. Here's one example:

/// <summary>
///A test for CloseCurrentTextLogFile
///</summary>
[TestMethod()]
public void CloseCurrentTextLogFileTest()
{
  Logger.CloseCurrentTextLogFile();
  Assert.Inconclusive( "A method that does not return a value cannot be verified." );
}

Based on the assert string, I'm wondering how to test this... Any ideas?

O.O
  • 11,077
  • 18
  • 94
  • 182
  • 3
    What would you want to validate? You could, for instance, assert specific conditions about the properties of the Logger class, i.e., that the current text log file is in the closed state and has the correct data in it. – mellamokb Jul 06 '12 at 16:52
  • 2
    Possible answer - http://stackoverflow.com/questions/5817234/create-unit-test-cases-for-a-method-which-does-not-return-any-value-in-c-sharp – JohnP Jul 06 '12 at 16:53
  • It looks like you are trying to test a method that closes a file handle. If that is the case, try to open the file for write after you call the method and try to write to it. Open it with something other than your Logger. Then close it back. If you can write to it, your method succeded. – Alex Mendez Jul 06 '12 at 16:54
  • It seems related to IO. IO errors are most of the time throws exception. Test that your method does not throw any Exception. – tschmit007 Jul 06 '12 at 16:56
  • possible duplicate of [Unit testing void methods?](http://stackoverflow.com/questions/246038/unit-testing-void-methods) – Alexei Levenkov Jul 06 '12 at 17:04

5 Answers5

3

I would posit that if it's truly not testable, then it doesn't actually do anything at all, and shouldn't exist ;) Something along the lines of this might work...

Assert.IsNotNull( Logger.File );
Logger.CloseCurrentTextLogFile();
Assert.IsNull( Logger.File );

Or check the status of Logger.FileOpenStatus or check that Logger.OpenFile(fname) throws an exception before closing, but not after. There's got to be something in Logger whose behaviour depends on whatever action CloseCurrentTextLogFile() performs.

mo.
  • 4,165
  • 3
  • 34
  • 45
3

Static state methods naturally make themselves fairly untestable, so my suggestion is based around refactoring your code away from static methods.

I would turn Logger into an instance class that takes an IO object in the constructor. That will allow you to stub the IO object and you can Assert that your IO object's Close method was called.

This is only if you want to make your code 100% testable. Otherwise, I would agree with Mo that if it is not testable, then do not write a forced test...those tend to be very brittle. In the end, you need to be pragmatic about your code. Often a logger is useful to keep static, however as I already mentioned, these tend to be very untestable....so just be pragmatic about your work and don't write tests in the mere effort to get 100% code coverage...that 100% will come with a price...

UPDATE

Here is why this is not truly testable from a dogmatic POV of unit testing. You are not testing a unit of work, but instead you are testing the Logger AND the Logger's dependencies (the IO object in this case). It also makes your tests slower and requiring environmental setup and state (you must have first opened an actual file to close it, right?). These are all bad for unit testing, but ok for integration testing...so it depends on what kind of tests you are writing, also.

Justin Pihony
  • 66,056
  • 18
  • 147
  • 180
  • +1 for the static methods comment. The `CloseCurrentTextLogFile` name implies static state and that alone will make testing extremely fragile. – Austin Salonen Jul 06 '12 at 17:07
  • So, if I understand your answer correctly, you recommend I get rid of static members because they are not easy to test? What's the point of static members if I can't use them? – O.O Jul 06 '12 at 17:16
  • @O.O Again, pragmatic coding is important here. If a static works for you, then use it. However, keep in mind that if you want to be STRICT to unit testing, then you cannot have any static methods. I just updated my answer to point out some of the problems with your code from a true UNIT testing perspective. I highly suggest The Art Of Unit Testing by Roy Osherove if you want an excellent understanding of unit testing. – Justin Pihony Jul 06 '12 at 17:24
  • @JustinPihony: I'd just like to clarify something. Being "strict" to unit testing means "you cannot have any static methods". Do you mean static methods that imply state (as it is here with files) or _all_ static methods? For example would a static method without state `static double ConvertDegreesToRadians(double radians)` still be acceptable under the "strict" definition of unit testing? – Chris Sinclair Jul 06 '12 at 17:49
  • 1
    @ChrisSinclair You are correct and thanks for pointing out my faux pas. In fact, here is an overflow question discussing just this fact: http://programmers.stackexchange.com/questions/5757/is-static-universally-evil-for-unit-testing-and-if-so-why-does-resharper-recom – Justin Pihony Jul 06 '12 at 18:10
2

You can check the state of Logger or you could call some other method on logger that will not produce an error because you called this method, which should succeeded if you had not called the method.

Sign
  • 1,919
  • 18
  • 33
2

Am not sure either, but you could try the following: A function is supposed to do something (write a file, set some variables, etc) Maybe you can check if the variables have been writen, or file has been created.

Josejulio
  • 1,286
  • 1
  • 16
  • 30
  • Yep, checking the file exists and has the contents you expect would be a good way to go. Remember, though, in general you don't want to put so much logic in your unit test that it needs its own testing! :) – mo. Jul 06 '12 at 17:03
  • i agree with you, it need to be as simple as possible. – Josejulio Jul 06 '12 at 17:07
1

You could mock up the Logger class and assert that the CloseCurrentTextLogFile is being called. Some might argue that you need to check that any open log files are closed, I personally do not agree with that as that would test the Logger itself not your method. This is the kind of questions developers should ask themselves when they start designing their systems, how can I test my application.

GETah
  • 20,922
  • 7
  • 61
  • 103
  • 2
    I think mocking the very object you're trying to test is not the way to go. All you'd be testing in this case is that your unit tests works, not that your `CloseCurrentTextLogFile` method works. – mo. Jul 06 '12 at 17:02
  • @mo. Good observation. However, testing that the logger is called is enough. Testing if the logger is doing the right thing is beyond this unit test. What you would test here is that your method is doing the right calls – GETah Jul 06 '12 at 17:04
  • 1
    I disagree. Again, all you're testing is that CloseCurrentTextLogFile **Test** () is calling `CloseCurrentTextLogFile()`. Your unit test should do something other than pat itself on the back and proclaim victory. – mo. Jul 06 '12 at 17:07
  • I agree with you but what you are describing here is called integration testing not unit testing. You want to make sure the logger is doing its job too and this should go to the logger class not here. – GETah Jul 06 '12 at 17:11