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.

Sign up for GitHub

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

Jump to bottom

Add more Array.Sort test combinations, per #41234 #41376

Closed
jeffhandley opened this issue Aug 26, 2020 · 5 comments · Fixed by #68489
Closed

Add more Array.Sort test combinations, per #41234 #41376

jeffhandley opened this issue Aug 26, 2020 · 5 comments · Fixed by #68489
Labels
area-System.Runtime good first issue Issue should be easy to implement, good for first-time contributors help wanted [up-for-grabs] Good issue for external contributors test-enhancement Improvements of test source code
Milestone
7.0.0

Comments

@jeffhandley
Copy link
Member

Should we have more test combinations, given this involves the complexity of pivots, etc?

Originally posted by @danmosemsft in #41234 (comment)
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Aug 26, 2020
@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@jeffhandley jeffhandley added area-System.Runtime test-enhancement Improvements of test source code labels Aug 26, 2020
@jeffhandley jeffhandley added this to the 6.0.0 milestone Aug 26, 2020
@jeffhandley jeffhandley mentioned this issue Aug 26, 2020
Merged
@ericstj ericstj removed the untriaged New issue has not been triaged by the area owner label Aug 26, 2020
@pgovind pgovind added the help wanted [up-for-grabs] Good issue for external contributors label Aug 10, 2021
@danmoseley danmoseley modified the milestones: 6.0.0, 7.0.0 Aug 12, 2021
@danmoseley danmoseley added the good first issue Issue should be easy to implement, good for first-time contributors label Aug 12, 2021
@jeffhandley jeffhandley mentioned this issue Feb 1, 2022
Closed
28 tasks
@ghost
Copy link

ghost commented Feb 6, 2022

Tagging subscribers to this area: @dotnet/area-system-collections
See info in area-owners.md if you want to be subscribed.

Issue Details

Should we have more test combinations, given this involves the complexity of pivots, etc?

Originally posted by @danmosemsft in #41234 (comment)

Author: jeffhandley
Assignees: -
Labels:

area-System.Collections, easy, test enhancement, up-for-grabs
Milestone: 7.0.0

@jeffhandley jeffhandley mentioned this issue Feb 6, 2022
Closed
4 tasks
@DDolG
Copy link
Contributor

DDolG commented Apr 20, 2022

Hello! I want to help. But I don't know how to start. I am new at the repo.
Could anyone point me in the right direction to start this thing?
I download the repository to my computer. Looked at what changes were added in the code. Looked at the added test case. I don't understand what is "complexity of pivots" means.

@krwq
Copy link
Member

krwq commented Apr 21, 2022

@DDolG you'll need to check the internal implementation details of the sort function and try to understand it. The particular piece mentioned in the other PR is referring to pivot from the intro sort - See https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/ArraySortHelper.cs#L473 - you will need to pick data which will exercise all branches of that code

@krwq krwq mentioned this issue Apr 25, 2022
Merged
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Apr 26, 2022
DDolG added a commit to DDolG/runtime that referenced this issue Apr 26, 2022
@DDolG
Fixes dotnet#41234 (dotnet#41376)
1b68d12 
*Added tests for sorting arrays

*Separated tests for integer from string

*Added comments for integers and strings
@ghost
Copy link

ghost commented Apr 28, 2022

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

Should we have more test combinations, given this involves the complexity of pivots, etc?

Originally posted by @danmosemsft in #41234 (comment)

Author: jeffhandley
Assignees: -
Labels:

area-System.Runtime, easy, test-enhancement, up-for-grabs, in-pr
Milestone: 7.0.0

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 24, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jul 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
area-System.Runtime good first issue Issue should be easy to implement, good for first-time contributors help wanted [up-for-grabs] Good issue for external contributors test-enhancement Improvements of test source code
Projects
None yet
Milestone
7.0.0
Development

Successfully merging a pull request may close this issue.

Add more array.sort test combination DDolG/runtime
7 participants
@krwq @jeffhandley @danmoseley @ericstj @pgovind @DDolG @Dotnet-GitSync-Bot