Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Commit 1f2f591

Browse files
grokysgrokys
committed
Get unit tests passing again.
- Explictly set current thread scheduler as `RxApp.MainThreadScheduler` in many tests. Seems that the behaviour of `MainThreadScheduler` has changed in some subtle way? - Explicitly clear `PullRequestDetailViewModel.OperationError` in the methods for each command. Seems `ReactiveCommand.IsExecuting` can now signal _after_ `ThrowExceptions`, making the error be immediately cleared. - Don't pass null commands to `Observable.Merge` in `LoginCredentialsViewModel`. It doesn't like that.
1 parent 608623a commit 1f2f591

File tree

14 files changed

+590
-447
lines changed

14 files changed

+590
-447
lines changed

‎GitHubVS.sln‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,9 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "GitHub.Exports.Reactive", "
6161
EndProject
6262
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Octokit", "Octokit", "{1E7F7253-A6AF-43C4-A955-37BEDDA01AC0}"
6363
EndProject
64-
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Octokit", "submodules\octokit.net\Octokit\Octokit.csproj", "{08DD4305-7787-4823-A53F-4D0F725A07F3}"
64+
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Octokit", "submodules\octokit.net\Octokit\Octokit.csproj", "{08DD4305-7787-4823-A53F-4D0F725A07F3}"
6565
EndProject
66-
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Octokit.Reactive", "submodules\octokit.net\Octokit.Reactive\Octokit.Reactive.csproj", "{674B69B8-0780-4D54-AE2B-C15821FA51CB}"
66+
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Octokit.Reactive", "submodules\octokit.net\Octokit.Reactive\Octokit.Reactive.csproj", "{674B69B8-0780-4D54-AE2B-C15821FA51CB}"
6767
EndProject
6868
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "ReactiveUI", "ReactiveUI", "{1E7F7253-A6AF-43C4-A955-37BEDDA01AB9}"
6969
EndProject
@@ -125,6 +125,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Splat", "submodules\splat\s
125125
EndProject
126126
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ReactiveUI.Events.WPF", "submodules\reactiveui\src\ReactiveUI.Events.WPF\ReactiveUI.Events.WPF.csproj", "{86C54B27-717F-478C-AC8C-01F1C68A56C5}"
127127
EndProject
128+
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ReactiveUI.Testing", "submodules\reactiveui\src\ReactiveUI.Testing\ReactiveUI.Testing.csproj", "{C6E8D1E1-FAAC-4E02-B6A1-6164EC5E704E}"
129+
EndProject
128130
Global
129131
GlobalSection(SolutionConfigurationPlatforms) = preSolution
130132
Debug|Any CPU = Debug|Any CPU
@@ -504,6 +506,16 @@ Global
504506
{86C54B27-717F-478C-AC8C-01F1C68A56C5}.Release|Any CPU.Build.0 = Release|Any CPU
505507
{86C54B27-717F-478C-AC8C-01F1C68A56C5}.ReleaseWithoutVsix|Any CPU.ActiveCfg = Release|Any CPU
506508
{86C54B27-717F-478C-AC8C-01F1C68A56C5}.ReleaseWithoutVsix|Any CPU.Build.0 = Release|Any CPU
509+
{C6E8D1E1-FAAC-4E02-B6A1-6164EC5E704E}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
510+
{C6E8D1E1-FAAC-4E02-B6A1-6164EC5E704E}.Debug|Any CPU.Build.0 = Debug|Any CPU
511+
{C6E8D1E1-FAAC-4E02-B6A1-6164EC5E704E}.DebugCodeAnalysis|Any CPU.ActiveCfg = Debug|Any CPU
512+
{C6E8D1E1-FAAC-4E02-B6A1-6164EC5E704E}.DebugCodeAnalysis|Any CPU.Build.0 = Debug|Any CPU
513+
{C6E8D1E1-FAAC-4E02-B6A1-6164EC5E704E}.DebugWithoutVsix|Any CPU.ActiveCfg = Debug|Any CPU
514+
{C6E8D1E1-FAAC-4E02-B6A1-6164EC5E704E}.DebugWithoutVsix|Any CPU.Build.0 = Debug|Any CPU
515+
{C6E8D1E1-FAAC-4E02-B6A1-6164EC5E704E}.Release|Any CPU.ActiveCfg = Release|Any CPU
516+
{C6E8D1E1-FAAC-4E02-B6A1-6164EC5E704E}.Release|Any CPU.Build.0 = Release|Any CPU
517+
{C6E8D1E1-FAAC-4E02-B6A1-6164EC5E704E}.ReleaseWithoutVsix|Any CPU.ActiveCfg = Release|Any CPU
518+
{C6E8D1E1-FAAC-4E02-B6A1-6164EC5E704E}.ReleaseWithoutVsix|Any CPU.Build.0 = Release|Any CPU
507519
EndGlobalSection
508520
GlobalSection(SolutionProperties) = preSolution
509521
HideSolutionNode = FALSE
@@ -534,6 +546,7 @@ Global
534546
{A4F579F3-77D3-450A-AACC-F2653EF11E69} = {1E7F7253-A6AF-43C4-A955-37BEDDA01AB9}
535547
{AD0306B7-F88E-44A4-AB36-1D04822E9234} = {1E7F7253-A6AF-43C4-A955-37BEDDA01AF9}
536548
{86C54B27-717F-478C-AC8C-01F1C68A56C5} = {1E7F7253-A6AF-43C4-A955-37BEDDA01AB9}
549+
{C6E8D1E1-FAAC-4E02-B6A1-6164EC5E704E} = {1E7F7253-A6AF-43C4-A955-37BEDDA01AB9}
537550
EndGlobalSection
538551
GlobalSection(ExtensibilityGlobals) = postSolution
539552
SolutionGuid = {556014CF-5B35-4CE5-B3EF-6AB0007001AC}

‎src/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModel.cs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,6 @@ protected override void Dispose(bool disposing)
555555
void SubscribeOperationError(ReactiveCommand<Unit, Unit> command)
556556
{
557557
command.ThrownExceptions.Subscribe(x => OperationError = x.Message);
558-
command.IsExecuting.Select(x => x).Subscribe(x => OperationError = null);
559558
}
560559

561560
static string GetBranchDisplayName(bool isFromFork, string owner, string label)
@@ -572,6 +571,8 @@ static string GetBranchDisplayName(bool isFromFork, string owner, string label)
572571

573572
IObservable<Unit> DoCheckout()
574573
{
574+
OperationError = null;
575+
575576
return Observable.Defer(async () =>
576577
{
577578
var localBranches = await pullRequestsService.GetLocalBranches(LocalRepository, Model).ToList();
@@ -597,6 +598,8 @@ IObservable<Unit> DoCheckout()
597598

598599
IObservable<Unit> DoPull()
599600
{
601+
OperationError = null;
602+
600603
return pullRequestsService.Pull(LocalRepository)
601604
.Do(_ =>
602605
{
@@ -609,6 +612,8 @@ IObservable<Unit> DoPull()
609612

610613
IObservable<Unit> DoPush()
611614
{
615+
OperationError = null;
616+
612617
return pullRequestsService.Push(LocalRepository)
613618
.Do(_ =>
614619
{
@@ -624,6 +629,7 @@ async Task DoSyncSubmodules()
624629
try
625630
{
626631
IsBusy = true;
632+
OperationError = null;
627633
usageTracker.IncrementCounter(x => x.NumberOfSyncSubmodules).Forget();
628634

629635
var result = await syncSubmodulesCommand.SyncSubmodules();

‎src/GitHub.Exports.Reactive/Validation/ReactivePropertyValidator.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ public ReactivePropertyValidator(IObservable<TProp> propertyChangeSignal)
177177
propertyChangeSignal
178178
.Select(x => new ValidationParameter { PropertyValue = x, RequiresReset = false })
179179
.Do(validationParameter => currentValidationParameter = validationParameter)
180-
.Subscribe(validationParameter => validateCommand.Execute(validationParameter));
180+
.Subscribe(validationParameter => validateCommand.Execute(validationParameter).Subscribe());
181181
}
182182

183183
public ReactivePropertyValidator<TProp> IfTrue(Func<TProp, bool> predicate, string errorMessage)

‎src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,10 @@ async Task RepoChanged(ILocalRepositoryModel localRepositoryModel)
192192
{
193193
await StatusChanged();
194194
}
195+
else
196+
{
197+
initialized.TrySetResult(null);
198+
}
195199
}
196200

197201
async Task StatusChanged()

‎src/GitHub.InlineReviews/ViewModels/NewInlineCommentThreadViewModel.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public NewInlineCommentThreadViewModel(ICommentService commentService,
5151
DeleteComment = ReactiveCommand.Create<Tuple<int, int>>(_ => { });
5252

5353
var placeholder = PullRequestReviewCommentViewModel.CreatePlaceholder(session, commentService, this, CurrentUser);
54-
placeholder.BeginEdit.Execute();
54+
placeholder.BeginEdit.Execute().Subscribe();
5555
this.WhenAnyValue(x => x.NeedsPush).Subscribe(x => placeholder.IsReadOnly = x);
5656
Comments.Add(placeholder);
5757

‎submodules/reactiveui‎

‎test/GitHub.App.UnitTests/GitHub.App.UnitTests.csproj‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,11 @@
2424

2525
<ItemGroup>
2626
<ProjectReference Include="..\..\src\GitHub.App\GitHub.App.csproj" />
27+
<ProjectReference Include="..\..\submodules\reactiveui\src\ReactiveUI.Testing\ReactiveUI.Testing.csproj" />
2728
</ItemGroup>
2829

2930
<ItemGroup>
30-
<PackageReference Include="Microsoft.Reactive.Testing" Version="4.0.0"/>
31+
<PackageReference Include="Microsoft.Reactive.Testing" Version="4.0.0" />
3132
<PackageReference Include="NSubstitute" Version="2.0.3" />
3233
<PackageReference Include="NUnit" version="3.9.0" />
3334
</ItemGroup>

‎test/GitHub.App.UnitTests/ViewModels/Dialog/Login2FaViewModelTests.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public async Task OkCommandCompletesAndReturnsNullWithNoAuthorizationCodeAsync()
4545
var userError = new TwoFactorRequiredUserError(exception);
4646
var task = target.Show(userError).ToTask();
4747

48-
target.OkCommand.Execute();
48+
target.OkCommand.Execute().Subscribe();
4949
var result = await task;
5050

5151
Assert.That(result, Is.Null);
@@ -60,7 +60,7 @@ public async Task OkCommandCompletesAndReturnsAuthorizationCodeAsync()
6060
var task = target.Show(userError).ToTask();
6161

6262
target.AuthenticationCode = "123456";
63-
target.OkCommand.Execute();
63+
target.OkCommand.Execute().Subscribe();
6464

6565
var result = await task;
6666
Assert.That("123456", Is.EqualTo(result.AuthenticationCode));
@@ -75,7 +75,7 @@ public async Task ResendCodeCommandCompletesAndReturnsRequestResendCodeAsync()
7575
var task = target.Show(userError).ToTask();
7676

7777
target.AuthenticationCode = "123456";
78-
target.ResendCodeCommand.Execute();
78+
target.ResendCodeCommand.Execute().Subscribe();
7979
var result = await task;
8080

8181
Assert.False(target.IsBusy);
@@ -91,7 +91,7 @@ public async Task ShowErrorMessageIsClearedWhenAuthenticationCodeSentAsync()
9191
var task = target.Show(userError).ToTask();
9292

9393
Assert.True(target.ShowErrorMessage);
94-
target.ResendCodeCommand.Execute();
94+
target.ResendCodeCommand.Execute().Subscribe();
9595

9696
var result = await task;
9797
Assert.False(target.ShowErrorMessage);

‎test/GitHub.App.UnitTests/ViewModels/Dialog/LoginCredentialsViewModelTests.cs‎

Lines changed: 55 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@
99
using NSubstitute;
1010
using ReactiveUI;
1111
using NUnit.Framework;
12+
using GitHub.ViewModels;
13+
using ReactiveUI.Testing;
14+
using System.Reactive.Concurrency;
1215

1316
public class LoginCredentialsViewModelTests
1417
{
@@ -17,72 +20,65 @@ public class TheDoneSignal : TestBaseClass
1720
[Test]
1821
public async Task SucessfulGitHubLoginSignalsDoneAsync()
1922
{
20-
var connectionManager = Substitute.For<IConnectionManager>();
21-
var connection = Substitute.For<IConnection>();
22-
23-
var gitHubLogin = Substitute.For<ILoginToGitHubViewModel>();
24-
var gitHubLoginCommand = ReactiveCommand.CreateFromObservable(() =>
25-
Observable.Return(connection));
26-
gitHubLogin.Login.Returns(gitHubLoginCommand);
27-
var enterpriseLogin = Substitute.For<ILoginToGitHubForEnterpriseViewModel>();
23+
using (TestUtils.WithScheduler(Scheduler.CurrentThread))
24+
{
25+
var connectionManager = Substitute.For<IConnectionManager>();
26+
var connection = Substitute.For<IConnection>();
2827

29-
var loginViewModel = new LoginCredentialsViewModel(connectionManager, gitHubLogin, enterpriseLogin);
30-
var signalled = false;
28+
var gitHubLogin = CreateLoginToHostViewModel<ILoginToGitHubViewModel>(connection);
29+
var enterpriseLogin = CreateLoginToHostViewModel<ILoginToGitHubForEnterpriseViewModel>();
30+
var loginViewModel = new LoginCredentialsViewModel(connectionManager, gitHubLogin, enterpriseLogin);
31+
var signalled = false;
3132

32-
loginViewModel.Done.Subscribe(_ => signalled = true);
33-
await gitHubLoginCommand.Execute();
33+
loginViewModel.Done.Subscribe(_ => signalled = true);
34+
await gitHubLogin.Login.Execute();
3435

35-
Assert.True(signalled);
36+
Assert.True(signalled);
37+
}
3638
}
3739

3840
[Test]
3941
public async Task FailedGitHubLoginDoesNotSignalDoneAsync()
4042
{
41-
var connectionManager = Substitute.For<IConnectionManager>();
43+
using (TestUtils.WithScheduler(Scheduler.CurrentThread))
44+
{
45+
var connectionManager = Substitute.For<IConnectionManager>();
4246

43-
var gitHubLogin = Substitute.For<ILoginToGitHubViewModel>();
44-
var gitHubLoginCommand = ReactiveCommand.CreateFromObservable(() =>
45-
Observable.Return<IConnection>(null));
46-
gitHubLogin.Login.Returns(gitHubLoginCommand);
47-
var enterpriseLogin = Substitute.For<ILoginToGitHubForEnterpriseViewModel>();
48-
49-
var loginViewModel = new LoginCredentialsViewModel(connectionManager, gitHubLogin, enterpriseLogin);
50-
var signalled = false;
47+
var gitHubLogin = CreateLoginToHostViewModel<ILoginToGitHubViewModel>();
48+
var enterpriseLogin = CreateLoginToHostViewModel<ILoginToGitHubForEnterpriseViewModel>();
49+
var loginViewModel = new LoginCredentialsViewModel(connectionManager, gitHubLogin, enterpriseLogin);
50+
var signalled = false;
5151

52-
loginViewModel.Done.Subscribe(_ => signalled = true);
53-
await gitHubLoginCommand.Execute();
52+
loginViewModel.Done.Subscribe(_ => signalled = true);
53+
await gitHubLogin.Login.Execute();
5454

55-
Assert.False(signalled);
55+
Assert.False(signalled);
56+
}
5657
}
5758

5859
[Test]
5960
public async Task AllowsLoginFromEnterpriseAfterGitHubLoginHasFailedAsync()
6061
{
61-
var connectionManager = Substitute.For<IConnectionManager>();
62-
var connection = Substitute.For<IConnection>();
63-
64-
var gitHubLogin = Substitute.For<ILoginToGitHubViewModel>();
65-
var gitHubLoginCommand = ReactiveCommand.CreateFromObservable(() =>
66-
Observable.Return<IConnection>(null));
67-
gitHubLogin.Login.Returns(gitHubLoginCommand);
68-
69-
var enterpriseLogin = Substitute.For<ILoginToGitHubForEnterpriseViewModel>();
70-
var enterpriseLoginCommand = ReactiveCommand.CreateFromObservable(() =>
71-
Observable.Return(connection));
72-
enterpriseLogin.Login.Returns(enterpriseLoginCommand);
73-
74-
var loginViewModel = new LoginCredentialsViewModel(connectionManager, gitHubLogin, enterpriseLogin);
75-
var success = false;
76-
77-
loginViewModel.Done
78-
.OfType<IConnection>()
79-
.Where(x => x != null)
80-
.Subscribe(_ => success = true);
81-
82-
await gitHubLoginCommand.Execute();
83-
await enterpriseLoginCommand.Execute();
84-
85-
Assert.True(success);
62+
using (TestUtils.WithScheduler(Scheduler.CurrentThread))
63+
{
64+
var connectionManager = Substitute.For<IConnectionManager>();
65+
var connection = Substitute.For<IConnection>();
66+
67+
var gitHubLogin = CreateLoginToHostViewModel<ILoginToGitHubViewModel>();
68+
var enterpriseLogin = CreateLoginToHostViewModel<ILoginToGitHubForEnterpriseViewModel>(connection);
69+
var loginViewModel = new LoginCredentialsViewModel(connectionManager, gitHubLogin, enterpriseLogin);
70+
var success = false;
71+
72+
loginViewModel.Done
73+
.OfType<IConnection>()
74+
.Where(x => x != null)
75+
.Subscribe(_ => success = true);
76+
77+
await gitHubLogin.Login.Execute();
78+
await enterpriseLogin.Login.Execute();
79+
80+
Assert.True(success);
81+
}
8682
}
8783
}
8884

@@ -118,4 +114,13 @@ public void LoginModeTracksAvailableConnections()
118114
Assert.That(LoginMode.EnterpriseOnly, Is.EqualTo(loginViewModel.LoginMode));
119115
}
120116
}
117+
118+
static T CreateLoginToHostViewModel<T>(IConnection login = null, IConnection oauthLogin = null)
119+
where T : class, ILoginToHostViewModel
120+
{
121+
var result = Substitute.For<T>();
122+
result.Login.Returns(ReactiveCommand.Create(() => login));
123+
result.LoginViaOAuth.Returns(ReactiveCommand.Create(() => oauthLogin));
124+
return result;
125+
}
121126
}

0 commit comments

Comments
 (0)