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

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

Page 41 of 49 (481 items) «3940414243»