-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Re-enable xUnit tests on Linux #850
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
Conversation
505bc10 to
8d831f3
Compare
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.
8d831f3 to
5c7fcbb
Compare
|
/cc @vors @daxian-dbw @SteveL-MSFT Waiting for review to merge. |
|
Looks fine to me, but I don't know much about ALC. I think @daxian-dbw should sign-up on that before merge. |
|
@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. |
|
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. |
|
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. |
|
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. |
|
|
I sign off the PS-ACL changes to unblock the xUnit test run. |
|
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. |
|
@vors should we disable the Reviewable check entirely or require the sign off to go through there? |
|
Let's disable it |
|
All right, it should be disabled going forward now. |
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