Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have a small little function in a VB6 codebase:

Public Function GetEquivalentCode(Optional ByVal code As String) As String

    If IsMissing(code) Or code = vbNullString Then code = m_GlobalCode

    Dim result As String

    If StringStartsWith("ABC", code) Then
        result = "ABC2013"
    Else
        result = code
    End If

    GetEquivalentCode = result

End Function

The implications of this function are massive, so before I deployed it I wrote a small test to ensure it returns the correct/expected value in all cases:

Public Sub GetEquivalentCodeTest()

    Dim result As String

    m_GlobalCode = "ABC2013"
    result = GetEquivalentCode
    Debug.Assert result = "ABC2013"

    m_GlobalCode = "ABC2014"
    result = GetEquivalentCode
    Debug.Assert result = "ABC2013"

    m_GlobalCode = "ABC2013A"
    result = GetEquivalentCode
    Debug.Assert result = "ABC2013"

    m_GlobalCode = "XYZ2013"
    result = GetEquivalentCode
    Debug.Assert result = "XYZ2013"

    m_GlobalCode = "SOMETHING ELSE"
    result = GetEquivalentCode
    Debug.Assert result = "SOMETHING ELSE"

    m_GlobalCode = vbNullString
    result = GetEquivalentCode
    Debug.Assert result = vbNullString

    Debug.Print "GetEquivalentCodeTest: Passed."

End Sub

Running this method from the immediate pane prints GetEquivalentCodeTest: Passed., and if I tweak it to make it fail, it breaks at the assertion that evaluates to False.

Is this the best way to write automated tests in VB6?


Colleagues call me a "purist" - I take it as a compliment. The thing is, without this test method, the mere fact that you give "ABC2014" to the function and it returns "ABC2013" can (will) be a major WTF for anyone maintaining this code in 2 years' time (including myself); the way I see it, these tests document what the [funky] business rules are.

Is there a way to write the test so as to make the business rules even more obvious?

share|improve this question
1  
I would make a single test case for every comportement you have! In my opinion it show that there is different rules (or a complex one). –  Marc-Andre Mar 5 at 17:52

4 Answers 4

up vote 5 down vote accepted

In my opinion, tests should be like a class, they should have only one responsibility. For a test, that would mean every time you want to show a special case, you should have a test method. The name that would represent what will be return or what the method is currently testing. Is a corner case, a typical case or just some examples ?

In your particular case every time you have something like :

m_GlobalCode = "ABC2013"
result = GetEquivalentCode
Debug.Assert result = "ABC2013"

would be a new test method.

I would add one more test method at least :

m_GlobalCode = "XYZ2014"
result = GetEquivalentCode
Debug.Assert result = "XYZ2014"

Why ? Because it would make clear that the method will change when the string start with ABC and nothing else.

share|improve this answer
    
he sort of already has that. the only difference is what I am assuming is the year. 2013 v 2014 –  Malachi Mar 5 at 18:57
    
Well just by looking at the tests (which is what I've done in the first place) I was thinking that 2014 was being changed to 2013. But in fact, it has nothing to do with the second part at all, the first part is what matters.By having two similar cases of the same format (three letters and 2013 || 2014) but with different result help me understand the rules. –  Marc-Andre Mar 5 at 19:05

The only thing that I can see at first glance to show what the business rule is, is to do something like this

m_GlobalCode = "ABC"
result = GetEquivalentCode
Debug.Assert result = "ABC2013"

probably at the beginning.

I think this is exactly what the business rule is saying from my understanding of the information that you have given


you probably want to add something like this to follow it, just to be clear what the rule is.

m_GlobalCode = "AB"
result = GetEquivalentCode
Debug.Assert result = "AB"

These two pieces coupled together show the entire Business Rule.

if the input string starts with "ABC" then return "ABC2013" otherwise return the input


6 tests should clearly define the Business Rule

  1. Input: "ABC"
  2. Input: "AB"
  3. Input: vbNullString
  4. Input: {Random String not Starting with "ABC"}
  5. Input: "DABC" Result: "DABC"
  6. Input: "ABC5029DDD"

Number 5 shows that ABC must be at the beginning of the string

Numbers 1 & 6 shows what the Function is doing.

Numbers 2,3,4, & 5 show what happens when the ABC is not at the beginning

share|improve this answer

As of now I see one major problem with this...

You are not even testing with the optional parameter.

All else equal, you are reusing the same variable and make multiple OPERATE statements.

"Each of the tests is clearly split into three parts. The first part builds up the test data, the second part operates on that test data, and the third part checks the operation yielded the expected results." - Robert C. Martin, Clean Code, p.127

Following this advice, you might want to split your test into neat compartments as mentioned by @Marc-Andre

share|improve this answer
1  
I agree and will +1 when I have ammo; one thing that's driving me insane is the m_GlobalCode variable which is Private to the code module it's in - the rest of the code really isn't testable at all. –  Mat's Mug Mar 5 at 18:31

Ok so this is what I ended up doing.

I created an interface, called it ITestClass.cls:

Option Explicit

Public Sub RunAllTests()
End Sub

Then I created a new module, called Tests.bas:

Private AllTests As Collection
Option Explicit

Private Sub SetupTestClasses()

    Set AllTests = New Collection

    AllTests.Add New PricingModuleTests
    'AllTests.Add New xxxxxxxxxTests

End Sub

Public Sub Run()

    SetupTestClasses

    Dim testClass As ITestClass
    For Each testClass In AllTests
        testClass.RunAllTests
    Next

End Sub

And then I added a new class to implement the ITestClass interface:

Implements ITestClass
Option Explicit

Private Sub ITestClass_RunAllTests()

    CodeShouldBeAsSpecified
    AnyABCCodeShouldBecomeABC2013

    Debug.Print TypeName(Me) & " completed."

End Sub


Public Sub CodeShouldBeAsSpecified()

    'arrange
    Dim code As String
    code = "XYZ123"

    Dim expected As String
    expected = "XYZ123"

    Dim actual As String

    'act
    actual = ModuleBeingTested.GetEquivalentCode(code)

    'assert
    Debug.Assert expected = actual
    Debug.Print "CodeShouldBeAsSpecified: PASS"

End Sub

Public Sub AnyABCCodeShouldBecomeABC2013()

    'arrange        
    Dim code As String
    code = "ABC123"

    Dim expected As String
    expected = "ABC2013"

    Dim actual As String

    'act
    actual = ModuleBeingTested.GetEquivalentCode(code)

    'assert
    Debug.Assert expected = actual
    Debug.Print "AnyABCCodeShouldBecomeABC2013: PASS"

End Sub

Now this has the drawback that I had to make the GetEquivalentCode method Public when it could have really been Private to ModuleBeingTested, but now I have a little bit of a structure so that whenever I write a piece of testable code (the rest of the code has too many dependencies to be anywhere near testable), I can write a test for it and have all tests execute with a single call.

As you can see, properly defining what the rules are makes much fewer tests required.

Immediate pane:

Tests.Run
CodeShouldBeAsSpecified: PASS
AnyABCCodeShouldBecomeABC2013: PASS
PricingModuleTests completed.

If any test fails, it won't output a "FAIL", rather the debugger will break at the assertion that failed; I'm fine with that.

share|improve this answer
    
and of course I can add more tests to make sure an unspecified optional parameter is handled the way it should be... but then I'll have to make more Private stuff Public in the ModuleBeingTested in order to control the dependencies. –  Mat's Mug Mar 5 at 19:33
    
Individual test methods could be Private, but that would rule out the possibility to run the tests individually. –  Mat's Mug Mar 5 at 19:40

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.