Reputation: 1132
What is the best way to write a Delphi DUnit test for a TThread descendant when FreeOnTerminate = True? The TThread descendant returns a reference which I need to test for, but I can't figure out how to wait for the thread to finish in the test...
unit uThreadTests;
interface
uses
Classes, TestFramework;
type
TMyThread = class(TThread)
strict private
FId: Integer;
protected
procedure Execute; override;
public
constructor Create(AId: Integer);
property Id: Integer read FId;
end;
TestTMyThread = class(TTestCase)
strict private
FMyId: Integer;
procedure OnThreadTerminate(Sender: TObject);
protected
procedure SetUp; override;
procedure TearDown; override;
published
procedure TestMyThread;
end;
implementation
{ TMyThread }
constructor TMyThread.Create(AId: Integer);
begin
FreeOnTerminate := True;
FId := AId;
inherited Create(False);
end;
procedure TMyThread.Execute;
begin
inherited;
FId := FId + 1;
end;
{ TestTMyThread }
procedure TestTMyThread.TestMyThread;
//var
// LThread: TMyThread;
begin
// LThread := TMyThread.Create(1);
// LThread.OnTerminate := OnThreadTerminate;
// LThread.WaitFor;
// CheckEquals(2, FMyId);
// LThread.Free;
///// The above commented out code is only useful of FreeOnTerminate = False;
with TMyThread.Create(1) do
begin
OnTerminate := OnThreadTerminate;
WaitFor; /// Not sure how else to wait for the thread to finish?
end;
CheckEquals(2, FMyId);
end;
procedure TestTMyThread.OnThreadTerminate(Sender: TObject);
begin
FMyId := (Sender as TMyThread).Id;
end; /// When FreeOnTerminate = True - THIS LINE CAUSES ERROR: Thread Error the handle is invalid
procedure TestTMyThread.SetUp;
begin
inherited;
end;
procedure TestTMyThread.TearDown;
begin
inherited;
end;
initialization
RegisterTests([TestTMyThread.Suite]);
end.
Any ideas would be welcomed.
Delphi 2010.
Upvotes: 13
Views: 5176
Reputation: 34899
Here is an example using an anonymous thread.
The important thing here is that the event is waited for in a thread. No dead-lock situation.
Uses
SyncObjs;
type
TMyThread = class(TThread)
private
FId : Integer;
protected
procedure Execute; override;
public
constructor Create( anInt : Integer);
property Id : Integer read FId;
end;
TestTMyThread = class
strict private
FMyId: Integer;
FMyEvent : TSimpleEvent;
procedure OnThreadTerminate(Sender: TObject);
protected
public
procedure TestMyThread;
end;
{ TMyThread }
constructor TMyThread.Create(anInt : Integer);
begin
inherited Create(True);
FreeOnTerminate := True;
FId := anInt;
end;
procedure TMyThread.Execute;
begin
Inc(FId);
end;
procedure TestTMyThread.TestMyThread;
var
AnonThread : TThread;
begin
FMyEvent := TSimpleEvent.Create(nil,true,false,'');
try
AnonThread :=
TThread.CreateAnonymousThread(
procedure
begin
With TMyThread.Create(1) do
begin
OnTerminate := Self.OnThreadTerminate;
Start;
end;
FMyEvent.WaitFor; // Wait until TMyThread is ready
end
);
AnonThread.FreeOnTerminate := False;
AnonThread.Start;
AnonThread.WaitFor; // Wait here until test is ready
AnonThread.Free;
Assert(FMyId = 2); // Check result
finally
FMyEvent.Free;
end;
end;
procedure TestTMyThread.OnThreadTerminate(Sender: TObject);
begin
FMyId := (Sender as TMyThread).Id;
FMyEvent.SetEvent; // Signal TMyThread ready
end;
Update, since Delphi-2010 does not have an anonymous thread class, here is an alternative which you can implement:
Type
TMyAnonymousThread = class(TThread)
private
FProc : TProc;
protected
procedure Execute; override;
public
constructor Create(CreateSuspended,SelfFree: Boolean; const aProc: TProc);
end;
constructor TMyAnonymousThread.Create(CreateSuspended,SelfFree: Boolean;
const aProc: TProc);
begin
Inherited Create(CreateSuspended);
FreeOnTerminate := SelfFree;
FProc := aProc;
end;
procedure TMyAnonymousThread.Execute;
begin
FProc();
end;
Upvotes: 2
Reputation: 4622
Create the thread in a suspended state, then set the OnTerminate
and finally Resume
the thread.
In your test class, define a private boolean field FThreadDone
which is initialized with false
and set to true
by the OnTerminate
Eventhandler.
Also, your constructor logic is a bit dirty, as you should not initialize field prior to calling the inherited constructor.
So:
constructor TMyThread.Create(AId: Integer);
begin
inherited Create(true);
FreeOnTerminate := True;
FId := AId;
end;
...
procedure TestTMyThread.TestMyThread;
begin
FThreadDone := False;
with TMyThread.Create(1) do begin // Note: Thread is suspended...
OnTerminate := OnThreadTerminate;
// Resume; // ... and finally started here!
Start;
end;
While not FThreadDone do Application.ProcessMessages;
CheckEquals(2, FMyId);
end;
procedure TestTMyThread.OnThreadTerminate(Sender: TObject);
begin
FMyId := (Sender as TMyThread).Id;
FThreadDone := True;
end;
This should do the job.
EDIT: Corrected stupid corrections, tested, works.
Upvotes: 2
Reputation: 163267
Subclass the thread to make it more testable. TThread
and TObject
provide enough hooks that you can add sensing variables to observe that it reaches certain points with the states you want it to have.
I see three aspects to this particular class that you might wish to test:
Id
property based on the value sent to the constructor.Id
property in the new thread, not the thread that calls the constructor.All those things are testable from a subclass, but hard to test otherwise without making changes to the thread's interface. (All the other answers so far require changing the thread's interface, such as by adding more constructor arguments or by changing the way it starts itself. That can make the thread harder, or at least more cumbersome, to use in the real program.)
type
PTestData = ^TTestData;
TTestData = record
Event: TEvent;
OriginalId: Integer;
FinalId: Integer;
end;
TTestableMyThread = class(TMyThread)
private
FData: PTestData;
public
constructor Create(AId: Integer; AData: PTestData);
destructor Destroy; override;
procedure AfterConstruction; override;
end;
constructor TTestableMyThread.Create(AId: Integer; const AData: PTestData);
begin
inherited Create(AId);
FData := AData;
end;
destructor TestableMyThread.Destroy;
begin
inherited;
FData.FinalId := Id;
// Tell the test that the thread has been freed
FData.Event.SetEvent;
end;
procedure TTestableMyThread.AfterConstruction;
begin
FData.OriginalId := Id;
inherited; // Call this last because this is where the thread starts running
end;
Using that subclass, it's possible to write a test that checks the three qualities identified earlier:
procedure TestTMyThread.TestMyThread;
var
Data: TTestData;
WaitResult: TWaitResult;
begin
Data.OriginalId := -1;
Data.FinalId := -1;
Data.Event := TSimpleEvent.Create;
try
TTestableMyThread.Create(1, @Data);
// We don't free the thread, and the event is only set in the destructor,
// so if the event is signaled, it means the thread freed itself: That
// aspect of the test implicitly passes. We don't want to wait forever,
// though, so we fail the test if we have to wait too long. Either the
// Execute method is taking too long to do its computations, or the thread
// isn't freeing itself.
// Adjust the timeout based on expected performance of Execute.
WaitResult := Data.Event.WaitFor(5000);
case WaitResult of
wrSignaled: ; // This is the expected result
wrTimeOut: Fail('Timed out waiting for thread');
wrAbandoned: Fail('Event was abandoned');
wrError: RaiseLastOSError(Data.Event.LastError);
else Fail('Unanticipated error waiting for thread');
end;
CheckNotEquals(2, Data.OriginalId,
'Didn''t wait till Execute to calculate Id');
CheckEquals(2, Data.FinalId,
'Calculated wrong Id value');
finally
Data.Event.Free;
end;
end;
Upvotes: 4
Reputation: 612914
Because you made the thread free itself upon termination then you have asked it to destroy all traces of itself as soon as it is done. Since you cannot exert influence on when it finishes, it is wrong to refer to anything inside the thread after you start it.
The solutions proposed by other, namely asking the thread to signal you when it terminates, are good. I personally would probably elect to do it that way. If you use an event as a signal then you can wait on that event.
However, there is another way to do it.
Because you own the duplicated handle, rather than the thread, you are safe to wait on it. It seems a little more complicated, but I suppose it avoids creating an extra synchronization object where one is not needed. Note that I'm not advocating this approach over the approach of using an event to signal completion.
Anyway, here's a simple demonstration of the idea.
{$APPTYPE CONSOLE}
uses
SysUtils, Windows, Classes;
type
TMyThread = class(TThread)
protected
procedure Execute; override;
public
destructor Destroy; override;
end;
destructor TMyThread.Destroy;
begin
Writeln('I''m dead!');
inherited;
end;
procedure TMyThread.Execute;
begin
end;
var
DuplicatedHandle: THandle;
begin
with TMyThread.Create(True) do // must create suspended
begin
FreeOnTerminate := True;
Win32Check(DuplicateHandle(
GetCurrentProcess,
Handle,
GetCurrentProcess,
@DuplicatedHandle,
0,
False,
DUPLICATE_SAME_ACCESS
));
Start;
end;
Sleep(500);
Writeln('I''m waiting');
if WaitForSingleObject(DuplicatedHandle, INFINITE)=WAIT_OBJECT_0 then
Writeln('Wait succeeded');
CloseHandle(DuplicatedHandle);
Readln;
end.
Upvotes: 2