-
-
Notifications
You must be signed in to change notification settings - Fork 15
Added incomplete test coverage for bug 547. #549
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: stable
Are you sure you want to change the base?
Added incomplete test coverage for bug 547. #549
Conversation
…s to enqueue even when the recipient wasn't found.
…ng with Entity Framework.
…hen testing the Enqueue extension method.
… during transfers.
|
Updated this PR with the logic fix in PR #548. |
| mockStream | ||
| ); | ||
|
|
||
| Assert.IsEmpty(_backgroundJobClient.Jobs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking out loud, it could be the case later on that something does end up getting scheduled to occur after an upload is processed. Doing some kind of analytics has been on my mind for a while.
What do you think about taking the approach of verifying _backgroundJobClient.Enqueue was not called with parameters that would schedule a transfer notification?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I believe this is a bit over my head due to my lack of experience in the repository.
Since Enqueue is an extension method, I'm not sure how we could verify whether or not it was called. I believe it simply calls Create so long as the source of invocation isn't null, so we could technically build something into that, but there's no guarantee that Enqueue is what invoked it without looking at the stack trace (unless there's something critical missing from my current understanding).
|
This is impressive. I'll need to process the code for a bit. |
Thanks, and no worries! I can keep the branch around for as long as you need, though I might need a reminder to pull the latest changes from |
|
I'll spend some time trying to get this unit test to work. |
|
I remember why I stopped mocking in this project now... Here's an integration test that covers the spectrum of user configurations. It requires a running PostgreSQL database, which you can bring up via [TestFixture]
internal class UploadFileTransfer_Tests
{
private Setup _setup;
private WebApplicationFactory<Program> _factory;
private ICrypterApiClient _client;
private ITokenRepository _clientTokenRepository;
private DefaultCryptoProvider _cryptoProvider;
private Ed25519KeyPair _emailVerificationKeyPair;
[OneTimeSetUp]
public async Task OneTimeSetUp()
{
_setup = new Setup();
await _setup.InitializeRespawnerAsync();
_cryptoProvider = new DefaultCryptoProvider();
_emailVerificationKeyPair = _cryptoProvider.DigitalSignature.GenerateKeyPair();
ICryptoProvider mockCryptoProvider = Mocks.CreateDeterministicCryptoProvider(_emailVerificationKeyPair).Object;
IServiceCollection overrideServices = new ServiceCollection();
overrideServices.AddSingleton(mockCryptoProvider);
_factory = await Setup.SetupWebApplicationFactoryAsync(overrideServices);
(_client, _clientTokenRepository) = Setup.SetupCrypterApiClient(_factory.CreateClient());
}
[TearDown]
public async Task TearDown()
{
await _setup.ResetServerDataAsync();
}
[OneTimeTearDown]
public async Task OneTimeTearDown()
{
await _factory.DisposeAsync();
}
[TestCase(false, false, false)]
[TestCase(true, false, false)]
[TestCase(true, true, false)]
[TestCase(true, true, true)]
public async Task Upload_File_Transfer_Enqueues_Recipient_Notification_Based_On_Settings(bool userHasEmail, bool userEmailVerified, bool userWantsNotifications)
{
string emailAddress = userHasEmail
? TestData.DefaultEmailAdress
: null;
RegistrationRequest registrationRequest = TestData.GetRegistrationRequest(TestData.DefaultUsername, TestData.DefaultPassword, emailAddress);
var registrationResult = await _client.UserAuthentication.RegisterAsync(registrationRequest);
if (userHasEmail)
{
// Allow the background service to "send" the verification email and save the email verification data
await Task.Delay(5000);
}
if (userEmailVerified)
{
DataContext dataContext = _factory.Services.GetRequiredService<DataContext>();
UserEmailVerificationEntity verificationData = await dataContext.UserEmailVerifications
.Where(x => x.User.Username == TestData.DefaultUsername)
.FirstAsync();
string encodedVerificationCode = UrlSafeEncoder.EncodeGuidUrlSafe(verificationData.Code);
byte[] signedVerificationCode = _cryptoProvider.DigitalSignature.GenerateSignature(_emailVerificationKeyPair.PrivateKey, verificationData.Code.ToByteArray());
string encodedSignature = UrlSafeEncoder.EncodeBytesUrlSafe(signedVerificationCode);
VerifyEmailAddressRequest verificationRequest = new VerifyEmailAddressRequest(encodedVerificationCode, encodedSignature);
Either<VerifyEmailAddressError, Unit> verificationResult = await _client.UserSetting.VerifyUserEmailAddressAsync(verificationRequest);
Assert.True(verificationResult.IsRight);
}
LoginRequest loginRequest = TestData.GetLoginRequest(TestData.DefaultUsername, TestData.DefaultPassword, TokenType.Session);
var loginResult = await _client.UserAuthentication.LoginAsync(loginRequest);
await loginResult.DoRightAsync(async loginResponse =>
{
await _clientTokenRepository.StoreAuthenticationTokenAsync(loginResponse.AuthenticationToken);
await _clientTokenRepository.StoreRefreshTokenAsync(loginResponse.RefreshToken, TokenType.Session);
});
Assert.True(registrationResult.IsRight);
Assert.True(loginResult.IsRight);
if (userWantsNotifications)
{
Either<UpdateNotificationSettingsError, Unit> notificationRequest = await _client.UserSetting.UpdateNotificationPreferencesAsync(new UpdateNotificationSettingsRequest(true, true));
}
(Func<EncryptionStream> encryptionStreamOpener, byte[] keyExchangeProof) = TestData.GetDefaultEncryptionStream();
UploadFileTransferRequest request = new UploadFileTransferRequest(TestData.DefaultTransferFileName, TestData.DefaultTransferFileContentType, TestData.DefaultPublicKey, TestData.DefaultKeyExchangeNonce, keyExchangeProof, TestData.DefaultTransferLifetimeHours);
var result = await _client.FileTransfer.UploadFileTransferAsync(TestData.DefaultUsername, request, encryptionStreamOpener, false);
Assert.True(result.IsRight);
JobStorage hangfireJobStorage = _factory.Services.GetRequiredService<JobStorage>();
JobList<FetchedJobDto> hangfireQueuedJobs = hangfireJobStorage.GetMonitoringApi().FetchedJobs("default", 0, 10);
bool transferNotificationQueued = hangfireQueuedJobs
.Where(x => x.Value.Job.Method == typeof(HangfireBackgroundService).GetMethod(nameof(HangfireBackgroundService.SendTransferNotificationAsync)))
.Any();
Assert.AreEqual(userHasEmail && userEmailVerified && userWantsNotifications, transferNotificationQueued);
}
} |
This PR is a proposal to introduce an incomplete test fixture for bug #547.
I ran into complications with mocking the LINQ query used in the private method
GetRecipientIdAsync:Since we can't mock extension methods, and I'm not well versed in Entity Framework, I don't believe I'm the right fit to complete this particular test coverage in a timely manner. However, I didn't want the effort to go to waste if the repository can benefit from it in any way.
This does require one minor modification to
DataContextin that theUsersproperty must be virtualized (seen in commit 54a940a). However, this is based on my working through a lack of working experience with Entity Framework. I added this particular change alone so that it can easily be reverted if not needed.