0

I have a test defined like so:

[Test]
 [TestCase('TestSetAsInteger 001.', '0')]
 [TestCase('TestSetAsInteger 002.', '666')]
 procedure TestSetAsInteger(IntVal : integer);

And Implemented like so:

procedure TTestKeyValueList.TestSetAsInteger(IntVal: integer);
begin
  FKVL.AsInteger['testkey'] := IntVal;
  Assert.AreEqual(IntVal, FKVL.AsInteger['testkey']);
end;

The methods in FKVL are stubs, so they should fail. Problem is, they always pass.

Now, If I change the test implementation to:

procedure TTestKeyValueList.TestSetAsInteger(IntVal: integer);
begin
  FKVL.AsInteger['testkey'] := IntVal;
  Sleep(1);  //This can be anything really, I can call FKVL.Count or other some such
  Assert.AreEqual(IntVal, FKVL.AsInteger['testkey']);
end;

The test now works as intended. Weird. Am I doing something wrong here?

RED MONKEY
  • 3,025
  • 2
  • 21
  • 23
  • 2
    When you say stubs, you mean that `AsInteger` getter function does not return anything which means it will return whatever content is in the EAX CPU register that holds the result of the function. Accidentally, that will be the value in the `IntVal` and the Assertion will pass. Calling `Sleep(1)` or any other code will change the content of the register and when you get to the assertion it will no longer be equal to the `IntVal`. Even when you have stub function you should make sure their result is initialized to something. – Dalija Prasnikar Aug 27 '23 at 18:58
  • BTW, your question does not have proper [mcve]. You should include the minimized FKVL declaration, including the declarations of the getter and setters functions called from your unit test. – Dalija Prasnikar Aug 27 '23 at 19:02
  • That pretty much answers it, thank you. – RED MONKEY Aug 27 '23 at 19:05

1 Answers1

4

You haven't provided exact declaration for the FKVL instance class, so I will assume it looks like following:

  TKVL = class   
  private
    procedure SetAsInteger(Idx: string; Value: integer);
    function GetAsInteger(Idx: string): Integer;   public
    property AsInteger[Idx: string]: Integer read GetAsInteger write SetAsInteger;   
  end;

You said methods are stubs which means they are empty, so the result of a GetAsInteger function will be uninitialized.

function TKVL.GetAsInteger(Idx: string): Integer;
begin

end;

procedure TKVL.SetAsInteger(Idx: string; Value: Integer);
begin

end;

When you compile that code you will also get a compiler warning about uninitialized function result.

[dcc32 Warning] W1035 Return value of function 'TKVL.GetAsInteger' might be undefined

When some value is undefined, that means the actual content will be whatever value is left over at that memory location by previous code. More precisely, in case of integer function its result will be returned in EAX CPU register.

Accidentally, calling SetAsInteger with IntVal as one of the parameters in previous line will cause the content of the IntVal parameter will be in the EAX at the time when GetAsInteger value is returned and assertion will pass.

When you add some other code in between, the content in the EAX register can change to something else and assertion will fail if it does.

Because the behavior is undefined, not only adding code can change the behavior of the test. You can observe test passing when using 32-bit Windows compiler in debug mode, but if you compile it in the release mode, the test will fail because optimization will change generated code and content of EAX register will no longer pick up leftover IntVal value. Same will happen if you compile the code with 64-bit Windows compiler. You can even observe how number of failed tests change, because test case with 0 will pass more often.

To prevent such undefined behavior you should always initialize function result to some value. This can be any value you find appropriate, even invalid one in the context of your code.

function TKVL.GetAsInteger(Idx: string): Integer;
begin
  Result := -1;
end;
Dalija Prasnikar
  • 27,212
  • 44
  • 82
  • 159