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

.netstandard2.0 support? #64

Open
ikijano opened this issue Sep 7, 2022 · 4 comments
Open

.netstandard2.0 support? #64

ikijano opened this issue Sep 7, 2022 · 4 comments

Comments

@ikijano
Copy link

ikijano commented Sep 7, 2022

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
Copy link

RobThree commented Sep 8, 2022

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
Copy link
Author

ikijano commented Sep 8, 2022

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
Copy link

@ikijano Why don't you do a PR?

@ikijano
Copy link
Author

ikijano commented Sep 14, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants