OData / OData.Neo

111 stars 18 forks source link

Unit tests for exceptions are failing 'in harmony' (.NET 7) #39

Open geertdoornbos opened 1 year ago

geertdoornbos commented 1 year ago

Tests for exceptions are created using InternalMock. It's constructed using:

service.Mock(methodName).Throws(exception)

The InternalMock implementation depends on Harmony to patch private methods.

This patching fails in some situations:

but, running tests in test explorer ('Debug') succeeds

The cause seems to be originated in MonoMod.Common as mentioned in Harmony issue 504

Waiting for .NET 7 support might not be the way forward https://github.com/pardeike/Harmony/issues/504#issuecomment-1320613517

TehWardy commented 7 months ago

It's been an ongoing issue discussed in the standard community with the InternalMock library. recently though a solution to this problem was proposed in that library.

I think we just need to regroup the team and discuss progress on the two projects so we can rekindle that progress ... https://github.com/cjdutoit/PartialMock.Demo

robertmclaws commented 7 months ago

I feel like if we're having to mock things for testing then we need to ask if we're on the wrong track. I've been able to guarantee stability with Restier using extensive internal unit tests and clean, simple integration tests.

It's a problem easily solved by disallowing private methods and marking everything internal instead.

TehWardy commented 7 months ago

That's an interesting take. I'm not sure how that would fit with the standard though it's worth raising with @hassanhabib for a deeper discussion.

For me I previously raised a concern that testing private methods was effectively testing implementation details which in unit testing circles is often referred to as an anti-pattern behaviour.

Marking the methods internal instead actually solves the need to use Harmony at all.

I think there's some interesting "ethos" discussions to be had in this area that affects the entire standard community, I'd be interested to see what a poll might uncover if we reached enough people for it to be considered an accurate reflection of views.

You do have a point though ... f it feels like you're fighting the technology you're probably not doing something right and it should be considered a time to internally reflect then ask "am I doing this the right way".