Skip to content

Fixes #3935 - Removed decimal validation from IsFixedPointNumeric method #3942

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

Merged
merged 3 commits into from
Oct 13, 2021

Conversation

wbalbo
Copy link

@wbalbo wbalbo commented Sep 27, 2021

Fixes #3935 Because his validation is used by the IsFloatingPointNumeric method, since issue 3831.

…his validation is used by IsFloatingPointNumeric method, since issue 3831.
@dnfadmin
Copy link

dnfadmin commented Sep 27, 2021

CLA assistant check
All CLA requirements met.

@jnm2 jnm2 linked an issue Sep 28, 2021 that may be closed by this pull request
Copy link
Contributor

@jnm2 jnm2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Welcome and thanks for helping!

ℹ If you write the text Fixes #3935 or Closes #3935 somewhere in the PR body (top comment), GitHub creates a relationship between the issue and the PR and autocloses the issue when the PR merges.

Could you please add a test of IsFixedPointNumeric in https://github.com/nunit/nunit/blob/master/src/NUnitFramework/tests/Constraints/NumericsTest.cs that would fail if your change was reverted?

@rprouse
Copy link
Member

rprouse commented Sep 28, 2021

The MacOS build failures on Azure DevOps is a NuGet restore failure. You can ignore it for now. Hopefully it will be fixed next run when you add some unit tests.

@wbalbo wbalbo changed the title Removed decimal validation from IsFixedPointNumeric method Fixes #3935 - Removed decimal validation from IsFixedPointNumeric method Sep 28, 2021
@wbalbo
Copy link
Author

wbalbo commented Sep 28, 2021

Welcome and thanks for helping!

ℹ If you write the text Fixes #3935 or Closes #3935 somewhere in the PR body (top comment), GitHub creates a relationship between the issue and the PR and autocloses the issue when the PR merges.

Could you please add a test of IsFixedPointNumeric in https://github.com/nunit/nunit/blob/master/src/NUnitFramework/tests/Constraints/NumericsTest.cs that would fail if your change was reverted?

Thank you for the reply.

I will write a test for this method when I can.

Do you know how I can run this test from Visual Studio 2019 using Main method from Program.cs class, in the nunitlite-runner project?

I tried this but didn't worked:

static int Main(string[] args)
        {
            args = new string[1];
            args[0] = "nunit.framework.tests.dll test 'class == 'NUnit.Framework.Constraints.NumericsTests'";
            return new TextRunner().Execute(args);
        }

@wbalbo
Copy link
Author

wbalbo commented Oct 12, 2021

Sorry for the delay, I added a test that will fail if my previous change reverts.

Copy link
Member

@rprouse rprouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. You happy with this test @jnm2 ?

@jnm2
Copy link
Contributor

jnm2 commented Oct 12, 2021

Yes, once it passes. It looks like it's failing right now: https://nunit.visualstudio.com/NUnit/_build/results?buildId=4346&view=ms.vss-test-web.build-test-results-tab&runId=1050460&resultId=104023&paneView=debug

I have no idea what the Assert.Throws part is all about in the existing tests, either. Isn't it redundant?

@wbalbo
Copy link
Author

wbalbo commented Oct 13, 2021

Yes, once it passes. It looks like it's failing right now: https://nunit.visualstudio.com/NUnit/_build/results?buildId=4346&view=ms.vss-test-web.build-test-results-tab&runId=1050460&resultId=104023&paneView=debug

I have no idea what the Assert.Throws part is all about in the existing tests, either. Isn't it redundant?

hmm, strange, I'm having this error when I try to debug my test:

Executing test method: NUnit.Framework.Constraints.NumericsTests.FailsOnDecimalIsPartOfIsFixedPointNumericMethod
StreamJsonRpc.RemoteInvocationException: Object reference not set to an instance of an object.

And this:

RPC server exception:
System.NullReferenceException: Object reference not set to an instance of an object.

Can you guys make all the unit tests work from your local project? I'm having these generic errors.

Trying to figure out how to solve it.

Or the ONLY way to test is using console runner? If it is I need help on how to debug a specific test.

@rprouse rprouse merged commit baa39a9 into nunit:master Oct 13, 2021
@wbalbo wbalbo deleted the Issue3935 branch October 13, 2021 11:43
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

Successfully merging this pull request may close these issues.

Numerics.IsFixedPointNumeric should return false for decimals
4 participants