Being Cellfish

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

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

    2008 Advent Calendar December 19th

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

    With such good test names, the name of the setup-method embarrasses me. Let's change it!

  • Being Cellfish

    2008 Advent Calendar December 18th

    • 1 Comments
    
    
    1: public class Advent18 2: { 3: private FileUtilWithDelete SetUp(string content, bool readable) 4: { 5: FileUtilWithDelete file = new FileUtilWithDelete("SomeFile.txt"); 6: file.Create(content); 7: file.Readable = readable; 8: return file; 9: } 10:   11: [Fact] 12: public void ReadingAReadableFileReturnsFileContent() 13: { 14: using (FileUtilWithDelete file = SetUp("CONTENT", true)) 15: { 16: string content = file.Read(); 17: Assert.Equal<string>("CONTENT", content); 18: } 19: } 20:   21: [Fact] 22: public void ReadingAnUnreadableFileThrowsCorrectException() 23: { 24: using (FileUtilWithDelete file = SetUp("SHOULD NOT BE ABLE TO READ THIS", false)) 25: { 26: Assert.Throws<AccessViolationException>(() => { file.Read(); }); 27: } 28: } 29: }

    Now I like the names but they are hard to read. Why not add some space?

  • Being Cellfish

    2008 Advent Calendar December 17th

    • 2 Comments
    
    
    1: public class Advent17 2: { 3: private FileUtilWithDelete SetUp(string content, bool readable) 4: { 5: FileUtilWithDelete file = new FileUtilWithDelete("SomeFile.txt"); 6: file.Create(content); 7: file.Readable = readable; 8: return file; 9: } 10:   11: [Fact] 12: public void TestReadReadableFileReturnsFileContent() 13: { 14: using (FileUtilWithDelete file = SetUp("CONTENT", true)) 15: { 16: string content = file.Read(); 17: Assert.Equal<string>("CONTENT", content); 18: } 19: } 20:   21: [Fact] 22: public void TestReadUnreadableFileThrowsCorrectException() 23: { 24: using (FileUtilWithDelete file = SetUp("SHOULD NOT BE ABLE TO READ THIS", false)) 25: { 26: Assert.Throws<AccessViolationException>(() => { file.Read(); }); 27: } 28: } 29: }

    Starting all test methods with "Test" does not really add any value to the name either. Naming the tests something that sounds like a real sentence is something I prefer.

  • Being Cellfish

    2008 Advent Calendar December 16th

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

    This test code starts to look pretty decent, don't you think? Well I think it is time to address the naming of the tests again. Even though I know the circumstances I'm testing I think the name should include something about the expected result also. And "Failing" that we used before is not really a good expected result (but it is better than nothing which is the case now).

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

Page 41 of 49 (482 items) «3940414243»