Skip to content
This repository was archived by the owner on Dec 5, 2024. It is now read-only.

Commit 7989f5b

Browse files
shanashana
committed
Make sure tests don't hang. Fix raising finally calls.
1 parent d6acc47 commit 7989f5b

File tree

5 files changed

+114
-46
lines changed

5 files changed

+114
-46
lines changed

‎src/GitHub.Api/Tasks/DownloadTask.cs

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@ public DownloadTask(CancellationToken token,
4646
Name = nameof(DownloadTask);
4747
}
4848

49+
protected string BaseRunWithReturn(bool success)
50+
{
51+
return base.RunWithReturn(success);
52+
}
53+
4954
protected override string RunWithReturn(bool success)
5055
{
5156
var result = base.RunWithReturn(success);
@@ -172,10 +177,29 @@ public DownloadTextTask(CancellationToken token,
172177
Name = nameof(DownloadTextTask);
173178
}
174179

175-
protected override string RunDownload(bool success)
180+
protected override string RunWithReturn(bool success)
176181
{
177-
var result = base.RunDownload(success);
178-
return fileSystem.ReadAllText(result, Encoding.UTF8);
182+
var result = BaseRunWithReturn(success);
183+
184+
RaiseOnStart();
185+
186+
try
187+
{
188+
result = RunDownload(success);
189+
result = fileSystem.ReadAllText(result, Encoding.UTF8);
190+
}
191+
catch (Exception ex)
192+
{
193+
Errors = ex.Message;
194+
if (!RaiseFaultHandlers(ex))
195+
throw;
196+
}
197+
finally
198+
{
199+
RaiseOnEnd(result);
200+
}
201+
202+
return result;
179203
}
180204
}
181205
}

‎src/GitHub.Api/Tasks/TaskBase.cs

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -321,14 +321,8 @@ protected virtual void RaiseOnStart()
321321

322322
protected virtual void RaiseOnEnd()
323323
{
324-
var success = Task.Status != TaskStatus.Faulted;
325-
if (success)
326-
{
327-
328-
}
329324
OnEnd?.Invoke(this);
330-
// if it's the last task of the chain and all went well (otherwise finally has been called already)
331-
if (success && continuation == null)
325+
if (continuation == null)
332326
finallyHandler?.Invoke();
333327
//Logger.Trace($"Finished {ToString()}");
334328
}
@@ -344,8 +338,6 @@ protected virtual bool RaiseFaultHandlers(Exception ex)
344338
if (handled)
345339
break;
346340
}
347-
if (!handled)
348-
finallyHandler?.Invoke();
349341
return handled;
350342
}
351343

@@ -619,9 +611,8 @@ protected override void RaiseOnStart()
619611
protected virtual void RaiseOnEnd(TResult result)
620612
{
621613
OnEnd?.Invoke(this, result);
622-
if (Task.Status == TaskStatus.Faulted || continuation == null)
614+
if (continuation == null)
623615
finallyHandler?.Invoke(result);
624-
RaiseOnEnd();
625616
//Logger.Trace($"Finished {ToString()} {result}");
626617
}
627618

‎src/tests/IntegrationTests/Download/DownloadTaskTests.cs

Lines changed: 60 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public override void TestFixtureTearDown()
3333
}
3434

3535
[Test]
36-
public async Task TestDownloadTask()
36+
public void TestDownloadTask()
3737
{
3838
Logger.Info("Starting Test: TestDownloadTask");
3939

@@ -42,11 +42,32 @@ public async Task TestDownloadTask()
4242
var gitLfs = new UriString($"http://localhost:{server.Port}/git-lfs.zip");
4343
var gitLfsMd5 = new UriString($"http://localhost:{server.Port}/git-lfs.zip.MD5.txt");
4444

45-
var md5 = await new DownloadTextTask(TaskManager.Token, fileSystem, gitLfsMd5, TestBasePath)
46-
.StartAwait();
45+
var evtDone = new ManualResetEventSlim(false);
4746

48-
var downloadPath = await new DownloadTask(TaskManager.Token, fileSystem, gitLfs, TestBasePath)
49-
.StartAwait();
47+
string md5 = null;
48+
new DownloadTextTask(TaskManager.Token, fileSystem, gitLfsMd5, TestBasePath)
49+
.Finally(r => {
50+
md5 = r;
51+
evtDone.Set();
52+
})
53+
.Start();
54+
55+
evtDone.Wait(10000);
56+
evtDone.Reset();
57+
Assert.NotNull(md5);
58+
59+
string downloadPath = null;
60+
new DownloadTask(TaskManager.Token, fileSystem, gitLfs, TestBasePath)
61+
.Finally(r => {
62+
downloadPath = r;
63+
evtDone.Set();
64+
})
65+
.Start();
66+
67+
evtDone.Wait(10000);
68+
evtDone.Reset();
69+
70+
Assert.NotNull(downloadPath);
5071

5172
var md5Sum = fileSystem.CalculateFileMD5(downloadPath);
5273
md5Sum.Should().BeEquivalentTo(md5);
@@ -58,8 +79,15 @@ public async Task TestDownloadTask()
5879
fileSystem.FileDelete(downloadPath);
5980
fileSystem.WriteAllBytes(downloadPath, cutDownloadPathBytes);
6081

61-
downloadPath = await new DownloadTask(TaskManager.Token, fileSystem, gitLfs, TestBasePath)
62-
.StartAwait();
82+
new DownloadTask(TaskManager.Token, fileSystem, gitLfs, TestBasePath)
83+
.Finally(r => {
84+
downloadPath = r;
85+
evtDone.Set();
86+
})
87+
.Start();
88+
89+
evtDone.Wait(10000);
90+
evtDone.Reset();
6391

6492
var downloadHalfPathBytes = fileSystem.ReadAllBytes(downloadPath);
6593
Logger.Trace("File size {0} Bytes", downloadHalfPathBytes.Length);
@@ -90,7 +118,7 @@ public void TestDownloadFailure()
90118

91119
downloadTask.Start();
92120

93-
autoResetEvent.WaitOne();
121+
autoResetEvent.WaitOne(10000);
94122

95123
taskFailed.Should().BeTrue();
96124
exceptionThrown.Should().NotBeNull();
@@ -106,7 +134,17 @@ public void TestDownloadTextTask()
106134
var gitLfsMd5 = new UriString($"http://localhost:{server.Port}/git-lfs.zip.MD5.txt");
107135

108136
var downloadTask = new DownloadTextTask(TaskManager.Token, fileSystem, gitLfsMd5, TestBasePath);
109-
var result = downloadTask.Start().Result;
137+
138+
var autoResetEvent = new AutoResetEvent(false);
139+
string result = null;
140+
downloadTask
141+
.Finally(r => {
142+
result = r;
143+
autoResetEvent.Set();
144+
})
145+
.Start();
146+
147+
autoResetEvent.WaitOne(10000);
110148
result.Should().Be("105DF1302560C5F6AA64D1930284C126");
111149
}
112150

@@ -120,15 +158,15 @@ public void TestDownloadTextFailure()
120158
var downloadTask = new DownloadTextTask(TaskManager.Token, fileSystem, "https://ggggithub.com/robots.txt");
121159
var exceptionThrown = false;
122160

123-
try
124-
{
125-
var result = downloadTask.Start().Result;
126-
}
127-
catch (Exception e)
128-
{
129-
exceptionThrown = true;
130-
}
131-
161+
var autoResetEvent = new AutoResetEvent(false);
162+
downloadTask
163+
.Finally((b, exception) => {
164+
exceptionThrown = exception != null;
165+
autoResetEvent.Set();
166+
})
167+
.Start();
168+
169+
autoResetEvent.WaitOne(10000);
132170
exceptionThrown.Should().BeTrue();
133171
}
134172

@@ -163,7 +201,7 @@ public void TestDownloadFileAndHash()
163201
})
164202
.Start();
165203

166-
autoResetEvent.WaitOne();
204+
autoResetEvent.WaitOne(10000);
167205

168206
result.Should().BeTrue();
169207
exception.Should().BeNull();
@@ -203,12 +241,13 @@ public void TestDownloadShutdownTimeWhenInterrupted()
203241

204242
downloadGitTask.Start();
205243

206-
evtStop.WaitOne();
244+
evtStop.WaitOne(10000);
207245

208246
watch.Start();
209247
TaskManager.Dispose();
210-
evtFinally.WaitOne();
248+
evtFinally.WaitOne(10000);
211249
watch.Stop();
250+
212251
server.Delay = 0;
213252
server.Abort();
214253

‎src/tests/TestWebServer/HttpServer.cs

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
using System;
1+
using GitHub.Unity;
2+
using System;
23
using System.Collections.Generic;
34
using System.Diagnostics;
45
using System.IO;
@@ -61,20 +62,27 @@ public void Stop()
6162

6263
public void Start()
6364
{
64-
listener.Start();
65-
while (true)
65+
try
6666
{
67-
try
68-
{
69-
abort = false;
70-
var context = listener.GetContext();
71-
Process(context);
72-
}
73-
catch
67+
listener.Start();
68+
while (true)
7469
{
75-
break;
70+
try
71+
{
72+
abort = false;
73+
var context = listener.GetContext();
74+
Process(context);
75+
}
76+
catch
77+
{
78+
break;
79+
}
7680
}
7781
}
82+
catch (Exception ex)
83+
{
84+
Logging.GetLogger(GetType()).Error(ex);
85+
}
7886
}
7987

8088
public void Abort()

‎src/tests/TestWebServer/TestWebServer.csproj

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,12 @@
5252
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
5353
</Content>
5454
</ItemGroup>
55+
<ItemGroup>
56+
<ProjectReference Include="..\..\GitHub.Logging\GitHub.Logging.csproj">
57+
<Project>{bb6a8eda-15d8-471b-a6ed-ee551e0b3ba0}</Project>
58+
<Name>GitHub.Logging</Name>
59+
</ProjectReference>
60+
</ItemGroup>
5561
<Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
5662
<!-- To modify your build process, add your task inside one of the targets below and uncomment it.
5763
Other similar extension points exist, see Microsoft.Common.targets.

0 commit comments

Comments
 (0)