Skip to content
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

Improve code coverage DevicesClient V2 #3073

14 changes: 10 additions & 4 deletions iothub/service/src/Amqp/AmqpConnectionHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ internal class AmqpConnectionHandler : IDisposable

// The current delivery tag. Increments after each send operation to give a unique value.
private int _sendingDeliveryTag;

/// <summary>
/// Creates an instance of this class. Provided for unit testing purposes only.
/// </summary>
protected AmqpConnectionHandler()
schoims marked this conversation as resolved.
Show resolved Hide resolved
{ }

internal AmqpConnectionHandler(
IotHubConnectionProperties credential,
Expand All @@ -66,7 +72,7 @@ internal AmqpConnectionHandler(
/// Returns false otherwise.
/// </summary>
/// <returns>True if this connection, its sessions and its sessions' links are all open. False otherwise.</returns>
internal bool IsOpen => _connection != null
internal virtual bool IsOpen => _connection != null
&& _connection.State == AmqpObjectState.Opened
Copy link
Contributor Author

@schoims schoims Jan 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made IsOpen virtual to override the implementation for unit testing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend adding a comment explaining this, otherwise it might be confusing as to why a class that isn't being inherited from in the sdk has virtual methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure!

&& _cbsSession != null
&& _cbsSession.IsOpen()
Expand All @@ -78,7 +84,7 @@ internal AmqpConnectionHandler(
/// then opening all the required sessions and links.
/// </summary>
/// <param name="cancellationToken">The cancellation token.</param>
internal async Task OpenAsync(CancellationToken cancellationToken)
internal virtual async Task OpenAsync(CancellationToken cancellationToken)
{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made the method virtual to override the implementation for unit testing.

if (Logging.IsEnabled)
Logging.Enter(this, "Opening amqp connection.", nameof(OpenAsync));
Expand Down Expand Up @@ -187,7 +193,7 @@ internal async Task OpenAsync(CancellationToken cancellationToken)
/// Closes the AMQP connection. This closes all the open links and sessions prior to closing the connection.
/// </summary>
/// <param name="cancellationToken">The cancellation token.</param>
internal async Task CloseAsync(CancellationToken cancellationToken)
internal virtual async Task CloseAsync(CancellationToken cancellationToken)
{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made the method virtual to override the implementation for unit testing.

if (Logging.IsEnabled)
Logging.Enter(this, "Closing amqp connection.", nameof(CloseAsync));
Expand Down Expand Up @@ -233,7 +239,7 @@ internal async Task CloseAsync(CancellationToken cancellationToken)
/// </summary>
/// <param name="message">The message to send.</param>
/// <param name="cancellationToken">The cancellation token.</param>
internal async Task<Outcome> SendAsync(AmqpMessage message, CancellationToken cancellationToken)
internal virtual async Task<Outcome> SendAsync(AmqpMessage message, CancellationToken cancellationToken)
{
ArraySegment<byte> deliveryTag = GetNextDeliveryTag();
return await _workerSession.SendAsync(message, deliveryTag, cancellationToken).ConfigureAwait(false);
Expand Down
16 changes: 16 additions & 0 deletions iothub/service/src/Feedback/MessageFeedbackProcessorClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,22 @@ public class MessageFeedbackProcessorClient : IDisposable
protected MessageFeedbackProcessorClient()
{ }

/// <summary>
/// Creates an instance of this class. Provided for unit testing purposes only.
/// </summary>
internal MessageFeedbackProcessorClient(
string hostName,
IotHubConnectionProperties credentialProvider,
IotHubServiceClientOptions options,
RetryHandler retryHandler,
AmqpConnectionHandler amqpConnection)
schoims marked this conversation as resolved.
Show resolved Hide resolved
{
_hostName = hostName;
_credentialProvider = credentialProvider;
_internalRetryHandler = retryHandler;
_amqpConnection = amqpConnection;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created another constructor to mock AmqpConnectionHandler for unit testing

internal MessageFeedbackProcessorClient(
string hostName,
IotHubConnectionProperties credentialProvider,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,21 @@ public class FileUploadNotificationProcessorClient : IDisposable
protected FileUploadNotificationProcessorClient()
{ }

/// <summary>
/// Creates an instance of this class. Provided for unit testing purposes only.
/// </summary>
internal FileUploadNotificationProcessorClient(
schoims marked this conversation as resolved.
Show resolved Hide resolved
string hostName,
IotHubConnectionProperties credentialProvider,
RetryHandler retryHandler,
AmqpConnectionHandler amqpConnection)
{
_hostName = hostName;
_credentialProvider = credentialProvider;
_internalRetryHandler = retryHandler;
_amqpConnection = amqpConnection;
}

internal FileUploadNotificationProcessorClient(
string hostName,
IotHubConnectionProperties credentialProvider,
Expand Down
15 changes: 15 additions & 0 deletions iothub/service/src/Messaging/MessagesClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,21 @@ protected MessagesClient()
{
}

/// <summary>
/// Creates an instance of this class. Provided for unit testing purposes only.
/// </summary>
internal MessagesClient(
schoims marked this conversation as resolved.
Show resolved Hide resolved
string hostName,
IotHubConnectionProperties credentialProvider,
RetryHandler retryHandler,
AmqpConnectionHandler amqpConnection)
{
_hostName = hostName;
_credentialProvider = credentialProvider;
_internalRetryHandler = retryHandler;
_amqpConnection = amqpConnection;
}

internal MessagesClient(
string hostName,
IotHubConnectionProperties credentialProvider,
Expand Down
2 changes: 2 additions & 0 deletions iothub/service/src/Utilities/Logging.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@
using System;
using System.Collections;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Diagnostics.Tracing;
using System.Globalization;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;

namespace Microsoft.Azure.Devices
{
[ExcludeFromCodeCoverage]
internal sealed partial class Logging : EventSource
schoims marked this conversation as resolved.
Show resolved Hide resolved
{
/// <summary>The single event source instance to use for all logging.</summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
using FluentAssertions;
using Azure;

namespace Microsoft.Azure.Devices.Tests
namespace Microsoft.Azure.Devices.Tests.Configurations
{
[TestClass]
[TestCategory("Unit")]
Expand Down Expand Up @@ -104,7 +104,7 @@ public async Task ConfigurationClients_CreateAsync_NullConfigurationThrows()
s_retryHandler);

// act
Func<Task> act = async () => await configurationsClient.CreateAsync((Configuration)null).ConfigureAwait(false);
Func<Task> act = async () => await configurationsClient.CreateAsync(null).ConfigureAwait(false);

// assert
await act.Should().ThrowAsync<ArgumentNullException>().ConfigureAwait(false);
Expand Down Expand Up @@ -331,7 +331,7 @@ public async Task ConfigurationClients_SetAsync_NullConfigurationThrows()
s_retryHandler);

// act
Func<Task> act = async () => await configurationsClient.SetAsync((Configuration)null).ConfigureAwait(false);
Func<Task> act = async () => await configurationsClient.SetAsync(null).ConfigureAwait(false);

// assert
await act.Should().ThrowAsync<ArgumentNullException>().ConfigureAwait(false);
Expand Down Expand Up @@ -385,7 +385,7 @@ public async Task ConfigurationClients_DeleteAsync()
{
// arrange
string configurationId = Guid.NewGuid().ToString().ToLower(); // Configuration Id characters must be all lower-case.
var configuration= new Configuration(configurationId)
var configuration = new Configuration(configurationId)
{
Priority = 1,
Labels = new Dictionary<string, string>
Expand Down Expand Up @@ -438,7 +438,7 @@ public async Task ConfigurationClients_DeleteAsync_NullConfigurationIdThrows()
s_retryHandler);

// act
Func<Task> act = async () => await configurationsClient.DeleteAsync(null,false).ConfigureAwait(false);
Func<Task> act = async () => await configurationsClient.DeleteAsync(null, false).ConfigureAwait(false);

// assert
await act.Should().ThrowAsync<ArgumentNullException>().ConfigureAwait(false);
Expand Down Expand Up @@ -546,7 +546,7 @@ public async Task ConfigurationClients_ApplyConfigurationContentOnDeviceAsync_Em
s_retryHandler);

// act
Func<Task> act = async () => await configurationsClient.ApplyConfigurationContentOnDeviceAsync(String.Empty, null).ConfigureAwait(false);
Func<Task> act = async () => await configurationsClient.ApplyConfigurationContentOnDeviceAsync(string.Empty, null).ConfigureAwait(false);

// assert
await act.Should().ThrowAsync<ArgumentException>().ConfigureAwait(false);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using FluentAssertions;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace Microsoft.Azure.Devices.Tests.Feedback
{
[TestClass]
[TestCategory("Unit")]
public class MessageFeedbackProcessorClientTests
{
private const string HostName = "contoso.azure-devices.net";
private static readonly string s_connectionString = $"HostName={HostName};SharedAccessKeyName=iothubowner;SharedAccessKey=dGVzdFN0cmluZzE=";

private static IIotHubServiceRetryPolicy noRetryPolicy = new IotHubServiceNoRetry();
private static IotHubServiceClientOptions s_options = new IotHubServiceClientOptions
{
Protocol = IotHubTransportProtocol.Tcp,
RetryPolicy = noRetryPolicy
};

[TestMethod]
public async Task MessageFeedbackProcessorClient_OpenAsync_NotSettingMessageFeedbackProcessorThrows()
{
// arrange
using var serviceClient = new IotHubServiceClient(
s_connectionString,
s_options);

// act
Func<Task> act = async () => await serviceClient.MessageFeedback.OpenAsync().ConfigureAwait(false);

// assert
await act.Should().ThrowAsync<InvalidOperationException>();
}

[TestMethod]
public async Task MessageFeedbackProcessorClient_OpenAsync_Cancelled_ThrowsOperationCanceledException()
{
// arrange
using var serviceClient = new IotHubServiceClient(
s_connectionString,
s_options);

// act
var ct = new CancellationToken(true);
Func<Task> act = async () => await serviceClient.MessageFeedback.OpenAsync(ct);

// assert
await act.Should().ThrowAsync<OperationCanceledException>();
}

[TestMethod]
public async Task MessageFeedbackProcessorClient_CloseAsync_Cancelled_ThrowsOperationCanceledException()
{
// arrange
using var serviceClient = new IotHubServiceClient(
s_connectionString,
s_options);

// act
var ct = new CancellationToken(true);
Func<Task> act = async () => await serviceClient.MessageFeedback.CloseAsync(ct);

// assert
await act.Should().ThrowAsync<OperationCanceledException>();
}
}
}
Loading