Uh oh!
There was an error while loading. Please reload this page.
C#: Re-factor FeedManager to allow better unit testing. - #22468
Merged
michaelnebel merged 8 commits intoSep 1, 2026
Conversation
michaelnebelforce-pushed
the
csharp/refactorfeedmanager
branch
from
August 31, 2026 11:24
08d2f0e to
b31ae61CompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
michaelnebelforce-pushed
the
csharp/refactorfeedmanager
branch
from
August 31, 2026 13:21
98f0019 to
446e52eComparemichaelnebelforce-pushed
the
csharp/refactorfeedmanager
branch
from
August 31, 2026 13:23
446e52e to
b76f793Comparemichaelnebel
marked this pull request as ready for review
September 1, 2026 06:52
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The new tests compare unordered sets using order-sensitive assertions, making them unreliable.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 | csharp/extractor/Semmle.Extraction.Tests/FeedManager.cs — ExplicitFeeds is a set, but Assert.Equal compares these enumerables in iteration order.… |
What changed in this PR
Refactors C# dependency-feed handling behind injectable interfaces and adds FeedManager unit tests.
Changes:
- Introduces abstractions for file, proxy, and feed I/O dependencies.
- Separates raw feed selection from CLI argument construction.
- Adds FeedManager and restore-source tests.
| File | Description |
|---|---|
csharp/extractor/Semmle.Extraction.Tests/Runtime.cs | Uses the shared DotNetStub. |
csharp/extractor/Semmle.Extraction.Tests/FeedManager.cs | Adds FeedManager unit tests and stubs. |
csharp/extractor/Semmle.Extraction.Tests/DotNetStub.cs | Adds reusable .NET CLI stub behavior. |
csharp/extractor/Semmle.Extraction.Tests/DotNet.cs | Tests source argument generation. |
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/SourceGenerators/ResxGenerator.cs | Accepts the file-provider interface. |
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/SourceGenerators/RazorGenerator.cs | Accepts the file-provider interface. |
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/SourceGenerators/DotnetSourceGeneratorBase.cs | Stores the file-provider abstraction. |
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/PackagesConfigRestorer.cs | Converts selected feeds into NuGet arguments. |
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs | Uses abstract dependencies and raw feeds. |
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/IFileProvider.cs | Defines file-discovery operations. |
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/IFeedManagerIO.cs | Defines feed-related I/O operations. |
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/IDependabotProxy.cs | Defines proxy configuration access. |
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileProvider.cs | Implements IFileProvider. |
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FeedManagerIO.cs | Extracts filesystem and reachability logic. |
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FeedManager.cs | Supports injected dependencies and raw feed selection. |
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNetCliInvoker.cs | Uses the proxy interface. |
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs | Builds source flags from raw feed URLs. |
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs | Stores abstract dependency types. |
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs | Implements IDependabotProxy. |
Suppressed comments (6)
csharp/extractor/Semmle.Extraction.Tests/FeedManager.cs:96
InheritedFeedsis a set, butAssert.Equalmakes this assertion depend on its unspecified iteration order. Compare the members without imposing sequence order.
Assert.Equal([
csharp/extractor/Semmle.Extraction.Tests/FeedManager.cs:112
AllFeedsis anImmutableHashSet, so its iteration order is not part of the contract. This sequence comparison can fail for a correct set; use strict unordered equivalence.
Assert.Equal([
csharp/extractor/Semmle.Extraction.Tests/FeedManager.cs:131
ReachableFeedsis set-valued, butAssert.Equalcompares enumeration order. The hash-set order can vary independently of behavior, making this assertion flaky; compare unordered members.
Assert.Equal([
csharp/extractor/Semmle.Extraction.Tests/FeedManager.cs:148
ReachableExplicitFeedsis anImmutableHashSet; asserting sequence equality relies on an order the property does not guarantee. Use strict unordered equivalence.
Assert.Equal([
csharp/extractor/Semmle.Extraction.Tests/FeedManager.cs:164
ReachableFallbackFeedsis set-valued, so this sequence assertion can fail solely because hash iteration order differs. Compare the set contents without ordering them.
Assert.Equal([
csharp/extractor/Semmle.Extraction.Tests/FeedManager.cs:181
feedsToUseis explicitly converted to aHashSet, then compared as an ordered enumerable. Its iteration order is unspecified, so this can reject the correct members; use strict unordered equivalence.
Assert.Equal([
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+79
to
+83
| Assert.Equal([ | ||
| "https://example.com/registry1", | ||
| "https://example.com/registry2", | ||
| "https://feed.from/config" | ||
| ], actualFeeds); |
ContributorAuthor
There was a problem hiding this comment.
According to the documentation seen hereAssert.Equals uses SetEquals under the hood for hash-sets - so I believe that the current implementation is correct.
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In this PR we
This is in preparation for using feeds configured via the dependabot proxy as "base" feed(s) (instead of the current hardcoded public
nuget.orgfeed).DCA looks good.