-
-
Notifications
You must be signed in to change notification settings - Fork 380
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
.NET Standard tests #479
.NET Standard tests #479
Conversation
global.json
Outdated
@@ -0,0 +1,5 @@ | |||
{ | |||
"sdk": { | |||
"version": "2.0.0" |
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.
I reckon this raises the barrier to entry and should be left to your build script
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.
I agree, but as I just discovered a moment ago appveyor fails to run the tests without it, not sure how to get around that.
@@ -1,3 +1,4 @@ | |||
#if FullFramework |
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.
I did some digging, it looks like the problem we are facing is this:
https://github.com/dotnet/corefx/issues/21079
and is fixed in 2.1:
dotnet/coreclr#14696
It's also probably cleaner to keep the compiler constants to the functionality they toggle instead of being 1-1 with the tfm, cause it would be easy for someone to use this for something other than removing code that relies on symbols working for example.
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.
Added the stacktrace flag to .NET Standard 2.0 and all the tests passed
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.
It's also probably cleaner to keep the compiler constants to the functionality they toggle instead of being 1-1 with the tfm, cause it would be easy for someone to use this for something other than removing code that relies on symbols working for example
Agreed. Now I've got the tests working I can refine these flags.
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.
Nice. What's weird for me is that I pull the branch into a fresh folder (proper fresh), do a restore, build, and test, and it cannot get the filenames (which are required by the tests to load the source), and I can see that the symbols are loaded.
I'm 100% hitting bug dotnet/corefx#21079, but clearly it's not an issue in AppVeyor
😕
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.
I was hitting the same, but as per the recommendation, if you copy the .DiaSymReader..dll's from C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\MSBuild\15.0\Bin\Roslyn to the tests bin directory, this temporarily solves the line number issues, and all the netcoreapp2.0 tests pass.
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.
@EddieGarmon oh nice, I'll have to give that a try. Thanks for the info 👍
A couple of thoughts added @josephwoodward |
See replies @slang25 |
@slang25 Interested to pull this down on my Windows machine and see if it all builds/tests pass. It's all working here 😁 |
lol |
Migrate tests to .NET Standard
No description provided.