Bug 37655 - [v8] mark Array.prototype.sort tests which verify if sorting is stable as passing for Chromium
Summary: [v8] mark Array.prototype.sort tests which verify if sorting is stable as pas...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-15 06:28 PDT by anton muhin
Modified: 2010-04-15 13:28 PDT (History)
4 users (show)

See Also:


Attachments
First take (1.41 KB, patch)
2010-04-15 06:28 PDT, anton muhin
no flags Details | Formatted Diff | Diff
Changing the wording per David suggestion (1.65 KB, patch)
2010-04-15 09:36 PDT, anton muhin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description anton muhin 2010-04-15 06:28:06 PDT
Created attachment 53430 [details]
First take

Recently algorithm for sorting small arrays (<= 22 elements) was changed and now sorting for those arrays is stable.

Note that larger arrays are sorted with quick sort, so sorting is not stable for them.
Comment 1 David Levin 2010-04-15 08:33:57 PDT
I would update the comment in test_expectation.txt because it is very confusing to see 
"// V8 doesn't stable sort and we currently have no intention of
// changing this."
Followed by two passing tests which are marked as WONTFIX!

Reading your comment in this bug it completely makes sense.
Comment 2 anton muhin 2010-04-15 08:43:55 PDT
(In reply to comment #1)
> I would update the comment in test_expectation.txt because it is very confusing
> to see 
> "// V8 doesn't stable sort and we currently have no intention of
> // changing this."
> Followed by two passing tests which are marked as WONTFIX!
> 
> Reading your comment in this bug it completely makes sense.

:)

David, if you have any suggestions how to make it more obvious, I'd really appreciate that.
Comment 3 David Levin 2010-04-15 08:59:46 PDT
Perhaps:
"// V8 doesn't stable sort and we currently have no intention of
// changing this. The following test only happen to pass due to
// the current algorithm used for sorting small arrays. If larger
// arrays were used in the tests, they would fail."
Comment 4 anton muhin 2010-04-15 09:36:27 PDT
Created attachment 53442 [details]
Changing the wording per David suggestion
Comment 5 anton muhin 2010-04-15 09:37:23 PDT
(In reply to comment #3)
> Perhaps:
> "// V8 doesn't stable sort and we currently have no intention of
> // changing this. The following test only happen to pass due to
> // the current algorithm used for sorting small arrays. If larger
> // arrays were used in the tests, they would fail."

Thanks, David, changed as per your suggestion.
Comment 6 anton muhin 2010-04-15 12:04:31 PDT
Comment on attachment 53442 [details]
Changing the wording per David suggestion

Thanks a lot, David
Comment 7 WebKit Commit Bot 2010-04-15 13:28:37 PDT
Comment on attachment 53442 [details]
Changing the wording per David suggestion

Clearing flags on attachment: 53442

Committed r57666: <http://trac.webkit.org/changeset/57666>
Comment 8 WebKit Commit Bot 2010-04-15 13:28:43 PDT
All reviewed patches have been landed.  Closing bug.