Skip to content

Handle wraparound for Random and Distinct#4316

Merged
stevenaw merged 1 commit into
nunit:masterfrom
mr-russ:fix_4308
Mar 23, 2023
Merged

Handle wraparound for Random and Distinct#4316
stevenaw merged 1 commit into
nunit:masterfrom
mr-russ:fix_4308

Conversation

@mr-russ

@mr-russ mr-russ commented Mar 22, 2023

Copy link
Copy Markdown
Contributor

Closes #4308

When using signed numbers, it's possible to have a situation
where (big_num) - (-ve small) > type max, eg for int8
128 - -127 = 255, but the type max is 128.  So the overflow would
result in incorrect answers.
Comment thread src/NUnitFramework/testdata/RandomAttributeFixture.cs

@OsirisTerje OsirisTerje left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, ref comments

Comment thread src/NUnitFramework/testdata/RandomAttributeFixture.cs

@stevenaw stevenaw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM thanks for your contribution @mr-russ !
I've pulled the code and the new tests pass locally for me

@stevenaw stevenaw merged commit f5fbb0c into nunit:master Mar 23, 2023
@mr-russ mr-russ deleted the fix_4308 branch March 23, 2023 07:23
@stevenaw

Copy link
Copy Markdown
Member

@nunit/framework-team @OsirisTerje Any objections to this being backported to 3.13? It seems fairly safe

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.

Random attribute with Distinct and wide range causes test to disappear

3 participants