Skip to content

Conversation

@andyleejordan
Copy link
Member

@andyleejordan andyleejordan commented Apr 14, 2016

The latest xUnit packages fix the "could not resolve coreclr path" problem we were having. To resolve all dependencies, the cli-deps feed was replaced with the aspnet feeds.

However, the latest xUnit packages do not allow us to set the default AssemblyLoadContext. So we have to allow PowerShell to continue with a test harness's ALC.

We do not (yet) support running the tests on OS X, so we pretend they pass for now.

This resolves #806.


This change is Reviewable

The latest xUnit packages fix the "could not resolve coreclr path"
problem we were having. To resolve all dependencies, the cli-deps feed
was replaced with the aspnet feeds.

However, the latest xUnit packages do not allow us to set the default
AssemblyLoadContext.
We have to pretend they pass on OS X for now.
@andyleejordan andyleejordan added the WG-Quality-Test issues in a test or in test infrastructure label Apr 15, 2016
@andyleejordan
Copy link
Member Author

/cc @vors @paulcallen @daxian-dbw

@andyleejordan andyleejordan mentioned this pull request Apr 15, 2016
@andyleejordan
Copy link
Member Author

/cc @vors @daxian-dbw @SteveL-MSFT Waiting for review to merge.

@vors
Copy link
Collaborator

vors commented Apr 18, 2016

Looks fine to me, but I don't know much about ALC. I think @daxian-dbw should sign-up on that before merge.

@daxian-dbw
Copy link
Member

@andschwa core powershell assumes the PowerShellAssemblyLoadContext to be the default ALC. With this change the assumption is broken, and I don't know how we can trust core powershell running that way.

@andyleejordan
Copy link
Member Author

We can log a message for debugging later if we need. But under the test harness, the PowerShell ALC cannot be the default. I don't see this breaking PowerShell itself, as it would still run with its own ALC when outside the test harness.

@daxian-dbw
Copy link
Member

I think when testing core powershell, the tests should run in the same core powershell settings as when presented to the user. If they are running in a different core powershell settings, then it's hard to say if the test results are reflecting the true issues.

@andyleejordan
Copy link
Member Author

That is not currently possible under the available xUnit runner. If you can find a way, please do, but AFAICT the harness sets the default ALC before running any of our code.

@andyleejordan
Copy link
Member Author

However, the latest xUnit packages do not allow us to set the default AssemblyLoadContext. So we have to allow PowerShell to continue with a test harness's ALC.

@daxian-dbw
Copy link
Member

I sign off the PS-ACL changes to unblock the xUnit test run.
But I think this is a broader test infrastructure problem and thus opened issue #864

@andyleejordan
Copy link
Member Author

Thanks Dongbo! I agree, think the .NET team will be able to help us out here, but we need to get the runner back up first.

@andyleejordan
Copy link
Member Author

@vors should we disable the Reviewable check entirely or require the sign off to go through there?

@vors
Copy link
Collaborator

vors commented Apr 19, 2016

Let's disable it

@andyleejordan
Copy link
Member Author

All right, it should be disabled going forward now.

@andyleejordan andyleejordan merged commit a655b43 into master Apr 19, 2016
@andyleejordan andyleejordan deleted the andschwa/fix-xunit branch April 19, 2016 02:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WG-Quality-Test issues in a test or in test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix the xUnit runner

4 participants