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

Add support for .NET Core #162

Closed
ghost opened this issue Jul 19, 2016 · 40 comments

Comments

@ghost
Copy link

commented Jul 19, 2016

Self-explanatory, but would be very nice to have.

@droyad

This comment has been minimized.

Copy link

commented Sep 23, 2016

I had a detailed go at this, and came up empty. The main issue I had was that I could not find a way to get the current StackTrace. This meant that the discovery of Attributes is not possible.

Also without the StackTrace, the code can't get the source filename using StackFrame.GetFileName(), which is relied upon by the naming.

One work around would be to pass the calling method into Verify. Another is to parse the Environment.StackTrace string.

Alternatively the overload needed (StackTrace(bool)) will be in netstandard2.0, which is expected in Q4/Q1.

@isidore

This comment has been minimized.

Copy link
Member

commented Sep 29, 2016

I'd be interested in pairing on this. should we wait until 2.0 ?

Llewellyn Falco
www.approvaltests.com

On Fri, Sep 23, 2016 at 12:03 AM, Robert Wagner notifications@github.com
wrote:

I had a detailed go at this, and came up empty. The main issue I had was
that I could not find a way to get the current StackTrace. This meant
that the discovery of Attributes is not possible.

Also without the StackTrace, the code can't get the source filename using
StackFrame.GetFileName(), which is relied upon by the naming.

One work around would be to pass the calling method into Verify. Another
is to parse the Environment.StackTrace string.

Alternatively the overload needed (StackTrace(bool)) will be in
netstandard2.0, which is expected in Q4/Q1.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#162 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAAqerMzhjCOHUCYb6ZUM9Y0CYzOiaM-ks5qs3nSgaJpZM4JPeRz
.

@ghost

This comment has been minimized.

Copy link
Author

commented Sep 30, 2016

If a valid workaround is to just pass the calling method into Verify, then CallerMemberNameAttribute will do the job (https://msdn.microsoft.com/en-us/library/system.runtime.compilerservices.callermembernameattribute(v=vs.110).aspx).

@AndreyAkinshin

This comment has been minimized.

Copy link

commented Jan 14, 2017

What the current status of the issue? I really want to use ApprovalTests.Net in one of my .NET Core projects.

@isidore

This comment has been minimized.

Copy link
Member

commented Jan 14, 2017

@fdmrun thanks for letting me know about CallerMemberNameAttribute. It's not enough for this, but I'm sure I'll use it in the future.

@AndreyAkinshin unsure. I think we are still waiting on netstandard2.0 but I'd be happy to explore this with you via skype if you have time this weekend?

@AndreyAkinshin

This comment has been minimized.

Copy link

commented Jan 14, 2017

I'd be happy to explore this with you via skype if you have time this weekend?

@isidore, Ok, send me your skype login to e-mail (can be found in my profile).

@isidore

This comment has been minimized.

Copy link
Member

commented Jan 14, 2017

Looks like stacktrace is still an issue as of this moment.
I'm not currently working with .netcore; so can one of you ping me when this looks possible again?

thanks

@mscottreed

This comment has been minimized.

Copy link

commented Jan 15, 2017

[CallerMemberName] kinda makes stack tracing unnecessary. I created a Approvals lite for .NET Core, and I have used it in two projects already.

@jamesrcounts

This comment has been minimized.

Copy link
Contributor

commented Jan 16, 2017

@mscottreed might have meant [CallerFilePathAttribute]

@mscottreed

This comment has been minimized.

Copy link

commented Jan 16, 2017

@EamonNerbonne

This comment has been minimized.

Copy link
Contributor

commented Feb 1, 2017

Stack walking seems not to find the appropriate location in non-core (e.g. net46-targetting) assemblies that were compiled with the new .net core tooling.

The stack-trace walking code has always been flaky for me - if your build process ever moves around files, things can get tricky easily; and then theres the need to add [MethodImpl(MethodImplOptions.NoInlining)] all over the place.

I currently use this small helper to avoid all (at least, I hope?) stack-walking code in approvals.net:

class SaneNamer : IApprovalNamer
{
    public string SourcePath { get; set; }
    public string Name { get; set; }
}

public static void Verify(string text, object IGNORE_PAST_THIS = null, [CallerFilePath] string filepath = null, [CallerMemberName] string membername = null)
{
    var writer = WriterFactory.CreateTextWriter(text);
    var filename = Path.GetFileNameWithoutExtension(filepath);
    var filedir = Path.GetDirectoryName(filepath);
    var namer = new SaneNamer {Name = filename + "." + membername, SourcePath = filedir};
    var reporter = new DiffReporter();
    Approver.Verify(new FileApprover(writer, namer, true), reporter);
}

Personally, I'd like an API without stackwalking as the default. Not only is that easier to use (no MethodImpl to forget, no deployment/postbuild interactions), you could also more easily stick it in a loop. The current api makes all that very difficult because it's quite unclear how what aspects of the configuration are injected. E.g. I love use approvals for refactoring, and often it's handy to split large test runs by some discriminator (e.g. language, client, etc.) to keep things manageable in size and make em easier to interpret. And without stackwalking, the need for unit-testing-framework(-version) specific adapters largely dissapears too.


@mscottreed link?

@jamesrcounts

This comment has been minimized.

Copy link
Contributor

commented Feb 1, 2017

Agree with @EamonNerbonne stack walking should be retired. In modern versions of .NET it doesn't seem necessary anymore (although it might have been the only solution in the past). I'd be in favor of dropping support for old versions of .NET or making a new library to support modern .NET (and of course a Core version would be just that).

@droyad

This comment has been minimized.

Copy link

commented Feb 1, 2017

I've created Assent for our .NET Core approval needs. It's nowhere are comprehensive as ApprovalTests though. It could be uses as a basis for a .NET Core version of ApprovalTests.

@SeanFeldman

This comment has been minimized.

Copy link

commented Aug 18, 2017

What's the current state of this issue? In the original comments there was mentioning of netstandard2.0.
With netcore2.0 out (which which I assume people would run the tests) this should be doable.

@niemyjski

This comment has been minimized.

Copy link

commented Sep 25, 2017

I'm also interested in this.

@isidore

This comment has been minimized.

Copy link
Member

commented Sep 25, 2017

@SimonCropp & I are working on this, it's taking a lot of yak shaving...

@niemyjski

This comment has been minimized.

Copy link

commented Oct 6, 2017

@isidore Are you working in any specific branches? It shouldn't take that much to port to .net standard 2.0

@SeanFeldman

This comment has been minimized.

Copy link

commented Oct 7, 2017

It shouldn't take that much to port to .net standard 2.0

@niemyjski assumptions... our industry is full of those. Avoid adding more 😉

@niemyjski

This comment has been minimized.

Copy link

commented Oct 7, 2017

I’ve ported oauth2 lib, exceptionless net and foundatio libs in less than a week... some cases took me 2-3 hours.. I’m 4-5 days in and I almost have exceptionless server fully converted to asp.net core from all legacy so I really don’t think it would take much.......

@SeanFeldman

This comment has been minimized.

Copy link

commented Oct 7, 2017

niemyjski added a commit to exceptionless/Exceptionless that referenced this issue Oct 8, 2017
@droyad

This comment has been minimized.

Copy link

commented Oct 8, 2017

I had a crack a few months ago, it looked like much muchness would be required

@PaulDMendoza

This comment has been minimized.

Copy link

commented Oct 24, 2017

@isidore Any updates on this? I'm trying to convert a test project to .NET Core 2.0 from .NET 4.5 and just realized this thread is open still.

@isidore

This comment has been minimized.

Copy link
Member

commented Oct 24, 2017

@PaulDMendoza

This comment has been minimized.

Copy link

commented Nov 30, 2017

Any updates on this getting fixed?

@niemyjski

This comment has been minimized.

Copy link

commented Dec 21, 2017

Were we able to see if there are any blockers upgrading yet?

@dionfoster

This comment has been minimized.

Copy link
Contributor

commented Jan 18, 2018

Hey guys, just wanted to try find out the progress of this. Are there blockers from external dependencies? What problems are there in getting it to netcore? Are there fundamental differences or lack of support from the framework that Approvals relies on?

@PaulDMendoza

This comment has been minimized.

Copy link

commented Jan 23, 2018

I'm also still interested. Any updates?

@LeonidEfremov

This comment has been minimized.

Copy link

commented Feb 20, 2018

C'mon guys, we really need this update =))

@isidore

This comment has been minimized.

Copy link
Member

commented Feb 23, 2018

Just an update.
We've been working on this, ApprovalTests has lots of dependencies on many different and incompatible technologies. I've been teasing it apart with @SimonCropp but if people are wanting it sooner I'd suggest setting up time to pair with me. (I'm not currently working in .net core, so pairing is only realistic way I make time to do this)

@niemyjski

This comment has been minimized.

Copy link

commented Feb 23, 2018

@bhugot

This comment has been minimized.

Copy link

commented Mar 25, 2018

done something in #214

@kl1mm

This comment has been minimized.

Copy link

commented May 14, 2018

Still waiting for .NetCore release on Nuget? Is there possibility that it will be released short term?

@isidore

This comment has been minimized.

Copy link
Member

commented Jul 14, 2018

just an update. @SimonCropp & I have separated a bunch of the things from approvaltests.

Leaving just

    <Reference Include="PresentationCore" Pack="False" />
    <Reference Include="PresentationFramework" Pack="False" />
    <Reference Include="System.Management" Pack="False" />
    <Reference Include="WindowsBase" Pack="False" />
&
    <Reference Include="System" Pack="False" />
    <Reference Include="System.Configuration" Pack="False" />

preventing us from a core compliant package. It's extremely slow work, but we are doing it and progress is being made.

@kl1mm

This comment has been minimized.

Copy link

commented Sep 24, 2018

are you getting forward with the port to .NetStandard? Would be amazing to have this at nuget

@JoshSchreuder

This comment has been minimized.

Copy link

commented Oct 3, 2018

Just tried out 3.0.18 which has netstandard2.0 support and it seems to be working for my netcoreapp2.1 test project running on Windows :)

@niemyjski

This comment has been minimized.

Copy link

commented Oct 3, 2018

I'll try it out, I seen it has a dependency on the registry.. Any specifics on this, I'm not sure that will work xplat.

@SimonCropp

This comment has been minimized.

Copy link
Contributor

commented Oct 24, 2018

@niemyjski the registry ref is only to enable the WindowsRegistryAssert functionality, which you would not be going on xplat

closing this since netcore is now supported

@SimonCropp SimonCropp closed this Oct 24, 2018

@niemyjski

This comment has been minimized.

Copy link

commented Oct 24, 2018

Could this be added in a windows only specific package so we don't have to bring this in cross plat

@SimonCropp

This comment has been minimized.

Copy link
Contributor

commented Oct 24, 2018

In this case i dont se the ROI. What exactly is your concern?

@benbelow

This comment has been minimized.

Copy link

commented Jun 14, 2019

@SimonCropp Do you expect this to be cross platform compatible then? I'm having some issues with Alphaleonis.Win32.Filesystem.Path running on an Ubuntu CI server

EDIT: Realised I was using an older version that did still support .net core - updating to 4.0.0 has solved my issue :) sorry to bother you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.