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

Cannot mock the BlobBaseClient.CanGenerateSasUri property for testing #18224

Closed
DMEvans opened this issue Jan 27, 2021 · 4 comments
Closed

Cannot mock the BlobBaseClient.CanGenerateSasUri property for testing #18224

DMEvans opened this issue Jan 27, 2021 · 4 comments
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Attention Workflow: This issue is responsible by Azure service team. Storage Storage Service (Queues, Blobs, Files)

Comments

@DMEvans
Copy link

DMEvans commented Jan 27, 2021

When trying to unit test a class that provides SAS token URI's for blob access I'm struggling to be able to assert all of the necessary invocations on the BlobClient.

The following code example shows what I'm trying to do:

var blobClient = GetBlobClient(blobName);

if (!blobClient.CanGenerateSasUri)
{
    throw new InvalidOperationException(MSG_CANNOT_GENERATE_TOKENS);
}

var sasBuilder = ...

var sasUri = blobClient.GenerateSasUri(sasBuilder)

In my tests I want to ensure that the correct exception is thrown when "CanGenerateSasUri" evaluates to "false", and that the correct values are passed to "GenerateSasUri" when it evaluates to "true".

However, the source code shows that the "CanGenerateSasUri" property is not virtual, so it cannot be overridden by my mocking framework. However, ALL of the other public members of this class are marked as virtual except this one.

/// <summary>
/// Determines whether the client is able to generate a SAS.
/// If the client is authenticated with a <see cref="StorageSharedKeyCredential"/>.
/// </summary>
public bool CanGenerateSasUri => SharedKeyCredential != null;

So my question is two-fold...

  1. Is there a reason why this property (and the same property on several other client classes in the Azure.Storage namespace) cannot be made virtual so it can be overridden for testing?
  2. Is there a workaround that will allow me to control within an individual test whether "CanGenerateSasUri" will return "true" or "false"?
@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Jan 27, 2021
@jsquire jsquire added Client This issue points to a problem in the data-plane of the library. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team Service Attention Workflow: This issue is responsible by Azure service team. Storage Storage Service (Queues, Blobs, Files) labels Jan 27, 2021
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jan 27, 2021
@ghost
Copy link

ghost commented Jan 27, 2021

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @xgithubtriage.

Issue Details

When trying to unit test a class that provides SAS token URI's for blob access I'm struggling to be able to assert all of the necessary invocations on the BlobClient.

The following code example shows what I'm trying to do:

var blobClient = GetBlobClient(blobName);

if (!blobClient.CanGenerateSasUri)
{
    throw new InvalidOperationException(MSG_CANNOT_GENERATE_TOKENS);
}

var sasBuilder = ...

var sasUri = blobClient.GenerateSasUri(sasBuilder)

In my tests I want to ensure that the correct exception is thrown when "CanGenerateSasUri" evaluates to "false", and that the correct values are passed to "GenerateSasUri" when it evaluates to "true".

However, the source code shows that the "CanGenerateSasUri" property is not virtual, so it cannot be overridden by my mocking framework. However, ALL of the other public members of this class are marked as virtual except this one.

/// <summary>
/// Determines whether the client is able to generate a SAS.
/// If the client is authenticated with a <see cref="StorageSharedKeyCredential"/>.
/// </summary>
public bool CanGenerateSasUri => SharedKeyCredential != null;

So my question is two-fold...

  1. Is there a reason why this property (and the same property on several other client classes in the Azure.Storage namespace) cannot be made virtual so it can be overridden for testing?
  2. Is there a workaround that will allow me to control within an individual test whether "CanGenerateSasUri" will return "true" or "false"?
Author: DMEvans
Assignees: -
Labels:

Client, Service Attention, Storage, customer-reported, needs-team-attention, needs-triage, question

Milestone: -

@jsquire
Copy link
Member

jsquire commented Jan 27, 2021

Thank you for your feedback. Tagging and routing to the team best able to assist.

@amnguye
Copy link
Member

amnguye commented Jan 27, 2021

Hi, there's actually no reason why we didn't make this virtual. It was an oversight when we implemented the new feature. Thanks for catching this, we will update this in all our clients so that it can become mockable.

@amnguye
Copy link
Member

amnguye commented Feb 1, 2021

Just merged a PR to resolve the issue. The fix will be in the next release.

@amnguye amnguye closed this as completed Feb 1, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Attention Workflow: This issue is responsible by Azure service team. Storage Storage Service (Queues, Blobs, Files)
Projects
None yet
Development

No branches or pull requests

3 participants