Testing code with Debug.Assert

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?

-
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 '14 at 17:52

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.

-
he sort of already has that. the only difference is what I am assuming is the year. 2013 v 2014 – Malachi Mar 5 '14 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 '14 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

-

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

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.

-
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 '14 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 '14 at 19:40

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

-
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 '14 at 18:31

Your GetEquivalentCode procedure is flawed. The IsMissing() function requires a variant; it will not behave as you may imagine when passed a string variable. Optional parameters if not variants will be assigned their default values: passed or not, they will be there.

To understand the risk here try this:

Private Sub Form_Load()
Call TestIt
End Sub

Private Sub TestIt(Optional bKeepIt As Boolean)
If Not IsMissing(bKeepIt) Then
If bKeepIt Then
Debug.Print "saving data"
Else
Debug.Print "ok lets delete everything"
End If
End If
End Sub

-
Excellent catch! Welcome to CR! The code isn't technically broken because OP is checking for vbNullString, but certainly IsMissing() does nothing here. ++ – RubberDuck Oct 10 '14 at 22:54