Being Cellfish

Stuff I wished I've found in some blog (and sometimes did)

December, 2008

Change of Address
This blog has moved to blog.cellfish.se.
Posts
  • Being Cellfish

    2008 Advent Calendar December 15th

    • 0 Comments
    
    
    1: public class Advent15 2: { 3: private FileUtilWithDelete SetUpReadable(string content) 4: { 5: FileUtilWithDelete file = new FileUtilWithDelete("SomeFile.txt"); 6: file.Create(content); 7: return file; 8: } 9:   10: private FileUtilWithDelete SetUpUnreadable(string content) 11: { 12: FileUtilWithDelete file = SetUpReadable(content); 13: file.Readable = false; 14: return file; 15: } 16:   17: [Fact] 18: public void TestReadReadableFile() 19: { 20: using (FileUtilWithDelete file = SetUpReadable("CONTENT")) 21: { 22: string content = file.Read(); 23: Assert.Equal<string>("CONTENT", content); 24: } 25: } 26:   27: [Fact] 28: public void TestReadUnreadableFile() 29: { 30: using (FileUtilWithDelete file = SetUpUnreadable("SHOULD NOT BE ABLE TO READ THIS")) 31: { 32: Assert.Throws<AccessViolationException>(() => { file.Read(); }); 33: } 34: } 35: }

    The two setup methods does not really feel necessary. Why not add another parameter to it and control readability that way?

  • Being Cellfish

    2008 Advent Calendar December 14th

    • 0 Comments
    
    
    1: public class Advent14 2: { 3: private IFileUtil m_file; 4:   5: private void SetUpReadable(string content) 6: { 7: m_file = new FileUtilWithDelete(Path.GetTempFileName()); 8: m_file.Create(content); 9: } 10:   11: private void SetUpUnreadable(string content) 12: { 13: SetUpReadable(content); 14: m_file.Readable = false; 15: } 16:   17: [Fact] 18: public void TestReadReadableFile() 19: { 20: SetUpReadable("CONTENT"); 21: string content = m_file.Read(); 22: Assert.Equal<string>("CONTENT", content); 23: } 24:   25: [Fact] 26: public void TestReadUnreadableFile() 27: { 28: SetUpUnreadable("SHOULD NOT BE ABLE TO READ THIS"); 29: Assert.Throws<AccessViolationException>(() => { m_file.Read(); }); 30: } 31: }

    The use of the new FileUtilWithDelete-class makes the tests neater than before I think. But the member variable should also be removed since it is a legacy from the time when we used the unit test framework setup-method. And we need to create a unique file name in order to get rid of the Dispose method. The use of a temporary (unknown) name makes it hard to find the file if something goes wrong and it is left on the host. So I want to get rid of that as soon as possible too.

  • Being Cellfish

    2008 Advent Calendar December 13th

    • 0 Comments
    
    
    1: public class Advent13 : IDisposable 2: { 3: private IFileUtil m_file; 4:   5: private void SetUpReadable(string content) 6: { 7: m_file = new FileUtil("SomeFile.txt"); 8: m_file.Create(content); 9: } 10:   11: private void SetUpUnreadable(string content) 12: { 13: SetUpReadable(content); 14: m_file.Readable = false; 15: } 16:   17: public void Dispose() 18: { 19: m_file.Delete(); 20: } 21:   22: [Fact] 23: public void TestReadReadableFile() 24: { 25: SetUpReadable("CONTENT"); 26: string content = m_file.Read(); 27: Assert.Equal<string>("CONTENT", content); 28: } 29:   30: [Fact] 31: public void TestReadUnreadableFile() 32: { 33: SetUpUnreadable("SHOULD NOT BE ABLE TO READ THIS"); 34: Assert.Throws<AccessViolationException>(() => { m_file.Read(); }); 35: } 36: }

    So far we've used the unit test framework clean up mechanism to remove the test file after each test. I don't see anything wrong with that but another way to do it would be to introduce a new class that does this for us. Tomorrow I'll make use of this new class:

        public class FileUtilWithDelete : FileUtil, IDisposable
        {
            public FileUtilWithDelete(string path)
                : base(path)
            {
                
            }
    
            public void Dispose()
            {
                Delete();
            }
        }
  • Being Cellfish

    2008 Advent Calendar December 12th

    • 2 Comments
    
    
    1: public class Advent12 : IDisposable 2: { 3: private IFileUtil m_file; 4:   5: private void SetUp(string content) 6: { 7: m_file = new FileUtil("SomeFile.txt"); 8: m_file.Create(content); 9: } 10:   11: public void Dispose() 12: { 13: m_file.Delete(); 14: } 15:   16: [Fact] 17: public void TestReadReadableFile() 18: { 19: SetUp("CONTENT"); 20: string content = m_file.Read(); 21: Assert.Equal<string>("CONTENT", content); 22: } 23:   24: [Fact] 25: public void TestReadUnreadableFile() 26: { 27: SetUp("SHOULD NOT BE ABLE TO READ THIS"); 28: m_file.Readable = false; 29: Assert.Throws<AccessViolationException>(() => { m_file.Read(); }); 30: } 31: }

    Since the setup-method is called explicitly, why not have several setup-methods for different kinds of setup?

  • Being Cellfish

    2008 Advent Calendar December 11th

    • 0 Comments
    
    
    1: public class Advent11 : IDisposable 2: { 3: private IFileUtil m_file; 4:   5: private void SetUp(string content) 6: { 7: m_file = new FileUtil("SomeFile.txt"); 8: m_file.Create(content); 9: } 10:   11: public void Dispose() 12: { 13: m_file.Delete(); 14: } 15:   16: [Fact] 17: public void TestReadOK() 18: { 19: SetUp("CONTENT"); 20: string content = m_file.Read(); 21: Assert.Equal<string>("CONTENT", content); 22: } 23:   24: [Fact] 25: public void TestReadFails() 26: { 27: SetUp("SHOULD NOT BE ABLE TO READ THIS"); 28: m_file.Readable = false; 29: Assert.Throws<AccessViolationException>(() => { m_file.Read(); }); 30: } 31: }

    OK, now let's take a look at the test names again. TestReadOK is actually a pretty decent name since it says "Reading works". TestReadFails is not a good name since it does not say anything about why the read operation is expected to fail. Next refactoring step will just be a rename of the test methods.

  • Being Cellfish

    2008 Advent Calendar December 10th

    • 0 Comments
    
    
    1: public class Advent10 : IDisposable 2: { 3: private IFileUtil m_file; 4:   5: private void SetUp(string content) 6: { 7: m_file = new FileUtil("SomeFile.txt"); 8: m_file.Create(content); 9: } 10:   11: public void Dispose() 12: { 13: m_file.Delete(); 14: } 15:   16: [Fact] 17: public void TestReadOK() 18: { 19: SetUp("CONTENT"); 20: string content = ""; 21: Assert.DoesNotThrow(() => { content = m_file.Read(); }); 22: Assert.Equal<string>("CONTENT", content); 23: } 24:   25: [Fact] 26: public void TestReadFails() 27: { 28: SetUp("SHOULD NOT BE ABLE TO READ THIS"); 29: m_file.Readable = false; 30: Assert.Throws<AccessViolationException>(() => { m_file.Read(); }); 31: } 32: }

    Some people think a good rule of thumb is to have only one assert in each test. I think it is OK to have more if the assert adds value if an unexpected failure happens. In this case I think line 21 does not really add value. If an exception is thrown the framework will report the unexpected exception anyway so let's get rid of that one.

  • Being Cellfish

    2008 Advent Calendar December 9th

    • 0 Comments
    
    
    1: public class Advent9 : IDisposable 2: { 3: private IFileUtil m_file; 4:   5: private void SetUp(string content) 6: { 7: m_file = new FileUtil("SomeFile.txt"); 8: m_file.Create(content); 9: } 10:   11: public void Dispose() 12: { 13: m_file.Delete(); 14: } 15:   16: [Fact] 17: public void TestReadOK() 18: { 19: SetUp("CONTENT"); 20: Assert.DoesNotThrow(() => { m_file.Read(); }); 21: } 22:   23: [Fact] 24: public void TestReadFails() 25: { 26: SetUp("SHOULD NOT BE ABLE TO READ THIS"); 27: m_file.Readable = false; 28: Assert.Throws<AccessViolationException>(() => { m_file.Read(); }); 29: } 30: }

    So now that I got a setup-method that that accepts desired content I should probably use it to actually verify the content of the file that reads OK...

  • Being Cellfish

    2008 Advent Calendar December 8th

    • 0 Comments
    
    
    1: public class Advent8 : IDisposable 2: { 3: private IFileUtil m_file; 4:   5: private void SetUp() 6: { 7: m_file = new FileUtil("SomeFile.txt"); 8: m_file.Create("CONTENT"); 9: } 10:   11: public void Dispose() 12: { 13: m_file.Delete(); 14: } 15:   16: [Fact] 17: public void TestReadOK() 18: { 19: SetUp(); 20: Assert.DoesNotThrow(() => { m_file.Read(); }); 21: } 22:   23: [Fact] 24: public void TestReadFails() 25: { 26: SetUp(); 27: m_file.Readable = false; 28: Assert.Throws<AccessViolationException>(() => { m_file.Read(); }); 29: } 30: }

    Unfortunately I've reintroduced the fact that the actual content is not verified for TestReadOK. And using a constant isn't something I want to do again. But since I'm calling the setup method explicitly I can modify it to accept the desired content. Wouldn't that be neat?

  • Being Cellfish

    2008 Advent Calendar December 7th

    • 0 Comments
    
    
    1: public class Advent7 : IDisposable 2: { 3: private IFileUtil m_file; 4: private const string m_content = "CONTENT"; 5:   6: public Advent7() 7: { 8: m_file = new FileUtil("SomeFile.txt"); 9: m_file.Create(m_content); 10: } 11:   12: public void Dispose() 13: { 14: m_file.Delete(); 15: } 16:   17: [Fact] 18: public void TestReadOK() 19: { 20: string content = ""; 21: Assert.DoesNotThrow(() => { content = m_file.Read(); }); 22: Assert.Equal<string>(m_content, content); 23: } 24:   25: [Fact] 26: public void TestReadFails() 27: { 28: m_file.Readable = false; 29: Assert.Throws<AccessViolationException>(() => { m_file.Read(); }); 30: } 31: }

    Having the expected content in a constant member is pretty convenient but if the class grows and gets a lot of tests I'll have to scroll quite a bit to see the actual value. Personally I think you should have all the information needed for the test local in the test. Also I'm not a big fan of using the unit test framework "setup" mechanism since the reader of the test must know that the framework executes a certain setup-method. Even though that is probably true most of the time I think it is better to explicitly call the setup method and make it visible exactly how the test is set up.

  • Being Cellfish

    2008 Advent Calendar December 6th

    • 0 Comments
    
    
    1: public class Advent6: IDisposable 2: { 3: private IFileUtil m_file; 4:   5: public Advent6() 6: { 7: m_file = new FileUtil("SomeFile.txt"); 8: m_file.Create("CONTENT"); 9: } 10:   11: public void Dispose() 12: { 13: m_file.Delete(); 14: } 15:   16: [Fact] 17: public void TestReadOK() 18: { 19: Assert.DoesNotThrow(() => { m_file.Read(); }); 20: } 21:   22: [Fact] 23: public void TestReadFails() 24: { 25: m_file.Readable = false; 26: Assert.Throws<AccessViolationException>(() => { m_file.Read(); }); 27: } 28: }

    With some redundant code broken out into the common setup method (the default constructor if you're not familiar with Xunit.net) there is another thing that bothers me about the TestReadOK-test. We're not actually verifying the content of the file. Let's fix that in the next version of this test.

Page 2 of 3 (25 items) 123