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

Ensure that all subpackages are located correctly #549

Merged
merged 2 commits into from
Mar 3, 2019
Merged

Conversation

CharliePoole
Copy link
Member

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.

@CharliePoole
Copy link
Member Author

There appears to be an issue with AppVeyor regarding loading certain NuGet packages. Not sure what we can do about that.

@mikkelbu
Copy link
Member

mikkelbu commented Feb 20, 2019

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.

Copy link
Member

@ChrisMaddock ChrisMaddock left a 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);

Copy link
Member

Choose a reason for hiding this comment

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

Whoops!

@ChrisMaddock ChrisMaddock self-assigned this Feb 23, 2019
@ChrisMaddock ChrisMaddock removed their assignment Feb 23, 2019
Copy link
Member

@ChrisMaddock ChrisMaddock left a 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!

@ChrisMaddock ChrisMaddock merged commit 03fedb2 into master Mar 3, 2019
@ChrisMaddock ChrisMaddock deleted the issue-546 branch March 3, 2019 18:19
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.

Cannot run a project file using --process:Separate
3 participants