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

[AccountDeleter]Adding stubbed definition for missing DI objects. #8677

Merged
merged 1 commit into from
Jul 22, 2021

Conversation

ryuyu
Copy link
Contributor

@ryuyu ryuyu commented Jul 12, 2021

Stubs out a minimal FeatureFlagService (used by PackageService) to allow DI to proceed successfully.

Copy link
Contributor

@loic-sharma loic-sharma left a comment

Choose a reason for hiding this comment

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

Would it be possible to add a test that verifies the DI container doesn't have issues at runtime?


using NuGet.Services.Entities;
using System;
using System.Collections.Generic;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: The order of dependencies?

@ryuyu
Copy link
Contributor Author

ryuyu commented Jul 13, 2021

Would it be possible to add a test that verifies the DI container doesn't have issues at runtime?

How would i go about doing this? Just have a test that attempts to hydrate the full real objects? That would require a full configuration for test, which I'm not against, but would be more work. Or if there is a simpler way, this sounds like a great idea.

@loic-sharma
Copy link
Contributor

loic-sharma commented Jul 14, 2021

How would i go about doing this? Just have a test that attempts to hydrate the full real objects? That would require a full configuration for test, which I'm not against, but would be more work. Or if there is a simpler way, this sounds like a great idea.

Can you create a test that uses the IServiceProvider to create an instance of ISubscriptionProcessor<AccountDeleteMessage> (similar to this)? You'll likely need to extend account deleter's Job to expose the _serviceProvider.

@ryuyu
Copy link
Contributor Author

ryuyu commented Jul 22, 2021

I filed #8700 for adding this test. I had a bit of difficulty immediately. I'll look into pulling it in when possible.

@ryuyu ryuyu merged commit 7e88712 into dev Jul 22, 2021
@ryuyu ryuyu deleted the ryuyu-fix-accountdeleter branch March 16, 2024 02:24
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.

5 participants