amantinband / throw

A simple, fluent, extensible, and fully customizable library for throwing exceptions for projects using .NET 6+
MIT License
1.25k stars 81 forks source link

.netstandard2.0 support? #64

Open ikijano opened 2 years ago

ikijano commented 2 years ago

Looks a fantastic library and I like to use it in my work project but unfortunate I'm stuck in .net framework world so i'm unable to use this library :( Have you ever considered supporting .netstandard2.0?

I played a little bit library source code and I managed get it compile when targeting to netstandard2.0 with langversion 10 using help of IsExternalInit and Nullable packages and re-ordering of namespace and using statements on couple files because of stylecop. It looks like there is no actual show stoppers in library code only some tests needs slight modifications. When running tests "only" 207/442 tests fails on .net framework 4.8. Common pattern seems to be that ArgumentException message format is slightly different on .net framework vs .net core. Tests are assuming blindly .net core message format instead of using real message format implemented by runtime which I considered to be implementation bug. If runtime implementation ever changes all these test will fail.

In example following test will fail on .net framework 4.8:

    public void Throw_WhenCustomExceptionMessage_ShouldThrowArgumentExceptionWithCustomMessage()
    {
        // Arrange
        ExceptionCustomizations exceptionCustomizations = ParameterConstants.CustomMessage;

        // Act
        Action action = () => ExceptionThrower.Throw(ParameterConstants.ParamName, exceptionCustomizations);

        // Assert
        action.Should().ThrowExactly<ArgumentException>().WithMessage($"{ParameterConstants.CustomMessage} (Parameter '{ParameterConstants.ParamName}')");
    }

Message:

Expected exception message to match the equivalent of "custom message (Parameter 'paramName')", but "custom message
Parameter name: paramName" does not.

if I change test to following form (not assuming any message format implementation details) test will succeed:

        // Assert
         action.Should().ThrowExactly<ArgumentException>().WithMessage(new ArgumentException(ParameterConstants.CustomMessage, ParameterConstants.ParamName).Message);

test repo (no test modifications) https://github.com/ikijano/throw/tree/wip-netstandard2

RobThree commented 2 years ago

I agree - a library should support Netstandard 2.0 whenever possible IMHO. Also agree on relying on string literals isn't the way go, but on the other hand the suggested 'fix' doesn't actually test anything either; it just mirrors internal behavior.

ikijano commented 2 years ago

I 'fixed' most of test in my test repo. So far everything is good.

I think, I could live with my fork until official library supports netstandard2.0 or i can move on to .net6+ era.

Of course it would be nice if official library would use more predicted message checks in tests so back porting new features to unofficial lib would be simpler.

RobThree commented 2 years ago

@ikijano Why don't you do a PR?

ikijano commented 2 years ago

I will do when I finish test fixes and do some cleanups.