restrict C# nuget unit tests to local package feeds #9694
Conversation
brettfo
force-pushed
the
dev/brettfo/nuget-unit-local
branch
4 times, most recently
from
c3a8787
to
c5de48d
Compare
| @@ -201,6 +223,42 @@ public abstract class UpdateWorkerTestBase : TestBase | |||
| AssertContainsFiles(expectedResult, actualResult); | |||
| } | |||
|
|
|||
| public static async Task MockNuGetPackagesInDirectory(MockNuGetPackage[]? packages, string temporaryDirectory) | |||
There was a problem hiding this comment.
This is where the fake NuGet packages are written to a local feed and an appropriate NuGet.Config is created.
There was a problem hiding this comment.
If no packages parameter is specified, no local feed and no NuGet.Config file will be created, which will still allow tests to hit the network if they're still required. As soon as a test sets packages: [...] then all network access is cut off. I did this to allow for the most flexibility, but also as a way to ensure that a test that specifies packages won't accidentally pull something from api.nuget.org.
There was a problem hiding this comment.
I wonder if this method should be extracted somewhere more central, since it isn't Updater specific.
There was a problem hiding this comment.
I made it public so I could use it elsewhere (in fact, the discover tests use it), I just didn't have a good place to put it and I wanted to get this huge PR off of my box :)
| private XDocument? _nuspec; | ||
| private Stream? _stream; | ||
|
|
||
| public void WriteToDirectory(string localPackageSourcePath) |
There was a problem hiding this comment.
This is the interesting part about the fake NuGet packages; a directory is populated with some specific files (code comments inline) that allow the directory to be used as a NuGet package source.
brettfo
force-pushed
the
dev/brettfo/nuget-unit-local
branch
from
c5de48d
to
7354d12
Compare
| var parentDirInfo = new DirectoryInfo(parentDirectory); | ||
| var childDirInfo = new DirectoryInfo(childDirectory); | ||
|
|
||
| while (childDirInfo.Parent is not null) |
There was a problem hiding this comment.
I like this more thorough implementation, but would childDirInfo.FullName.StartsWith(parentDirInfo.FullName); work? This is me assuming DirectoryInfo normalized the paths.
There was a problem hiding this comment.
.StartsWith() doesn't work for things like C:\temp and C:\temporary-documents\file.txt. In this case .StartsWith will return true, but it's not under the same path. I considered ensuring a trailing directory separator, but that ends up with a bunch of edge cases that I didn't like.
| } | ||
|
|
||
| // override various nuget locations | ||
| foreach (var envName in new[] { "NUGET_PACKAGES", "NUGET_HTTP_CACHE_PATH", "NUGET_SCRATCH", "NUGET_PLUGINS_CACHE_PATH" }) |
There was a problem hiding this comment.
Do these values need to be reverted or is the expectation that every test will end up mocking their packages?
There was a problem hiding this comment.
They're re-set in every test and the only negative effect I could think of is if a test doesn't use the fake NuGet packages and actually wants to hit api.nuget.org, it's possible that these values will cause that test to break, but since the goal of this is to isolate the tests, that kind of test breaking is probably a good thing.
| @@ -201,6 +223,42 @@ public abstract class UpdateWorkerTestBase : TestBase | |||
| AssertContainsFiles(expectedResult, actualResult); | |||
| } | |||
|
|
|||
| public static async Task MockNuGetPackagesInDirectory(MockNuGetPackage[]? packages, string temporaryDirectory) | |||
There was a problem hiding this comment.
I wonder if this method should be extracted somewhere more central, since it isn't Updater specific.
raj-meka
force-pushed
the
dev/brettfo/nuget-unit-local
branch
from
7354d12
to
2b7a2d3
Compare
Comments have been added inline with the code where the most attention should be paid.
In short, if a unit test is written as it was before, the behavior will remain the same; network access to
api.nuget.orgwill be allowed. However, as soon as thepackagesparameter is specified, a test-onlyNuGet.Configis written to the test directory which disallows all network access and only allows the packages created in a special directory to be queried. Since we don't use any custom NuGet API handling, this is perfectly fine as we're letting the "real" NuGet handle its sources however it sees fit (which in this case is all from a local feed.)The vast majority of this PR was changing things like
Newtonsoft.JsontoSome.Packageand can be ignored. In some tests the specific package dependencies that made the test case interesting have been explicitly laid out which should help with readability quite a bit.