-
Notifications
You must be signed in to change notification settings - Fork 152
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
Ensure that all subpackages are located correctly #549
Conversation
There appears to be an issue with AppVeyor regarding loading certain NuGet packages. Not sure what we can do about that. |
The problem with nuget is the same as in #515 which should be solved by #525, but it seems like the solution is not enough. I'll see if I have time to look at it tonight. Edit: And AppVeyor had new images uploaded 11 February (see e.g. AppVeyor Windows images update. There was no changes to msbuild, but nuget was bumped. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Charlie - left one trivial comment, but I'm going to need to pull this down in VS to fully understand what's going on - which I'm not going to be in a position to do for a few days yet, I'm afraid.
var packages = new List<TestPackage>(TestPackage.SubPackages); | ||
if (packages.Count == 0) | ||
packages.Add(TestPackage); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Charlie - looks good. You comments from the other PR have helped me understand what's going on here a little better!
Fixes #546
This fix ensures that all leaf packages - those without subpackages - are handled in DirectRunner no matter how deep the hierarchy of packages and subpackages is.
It continues to pass all tests and solves the problem of not being able to run a project file using the --process:Separate option. However, I have not added a test for that case since it would require a project loader extension to be installed in order to function. This seems to be another one to deal with in the accepttance tests.
I intended to make a similar change in AggregatingTestRunner but discovered that the comparable code there for examining packages was not being used and removed it.