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

Added unit test for RedisCacheWrapper. #16787

Merged

Conversation

gvkries
Copy link
Contributor

@gvkries gvkries commented Sep 24, 2024

Fixes #16761

/cc @hishamco

@Piedone Piedone requested a review from hishamco September 24, 2024 18:57
@hishamco
Copy link
Member

I think I will do it but it's great that you picked up 😁

@hishamco
Copy link
Member

I will review you the PR tomorrow if not today

@gvkries
Copy link
Contributor Author

gvkries commented Sep 25, 2024

I think I will do it but it's great that you picked up 😁

Yeah, I saw that you're assigned to that issue. I was just thinking about how one may test something like this and wanted to share my approach. Feel free to close this PR, if you have a different or better way of doing it.🤗


public RedisCacheWrapper(IOptions<RedisCacheOptions> optionsAccessor) => _cache = new RedisCache(optionsAccessor);
public RedisCacheWrapper(IDistributedCache cache) => _cache = cache ?? throw new ArgumentNullException(nameof(cache));
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure why you changed the parameter type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because RedisCache cannot be easily mocked for unit testing.

@hishamco
Copy link
Member

@sebastienros do you have anything else to add here?

@hishamco hishamco enabled auto-merge (squash) October 3, 2024 18:07
@hishamco hishamco merged commit cff607a into OrchardCMS:main Oct 3, 2024
5 checks passed
@gvkries gvkries deleted the gvkries/unittest-rediscachewrapper-16761 branch January 7, 2025 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Unit Test for RedisCacheWrapper shouldn't implement IDisposable
3 participants