Nodejs collation tests: Change "fail" to "unsupported" when a rule or complex comparison is used. #321
Conversation
|
Note that the same should be done in Dart Web. And rules should be implemented in ICU4 C, J, and X, too. |
| } | ||
|
|
||
| if (result != true) { | ||
| if (result == true) { |
There was a problem hiding this comment.
Issue: if result == false, it should be returned as a potential test failure. Only if the options are unsupported should an unsupported status be returned.
There was a problem hiding this comment.
OK. I see that there's an ECMAScript option for sensitivity. I'll see if that fits with any of the "compare_type" options.
And should we set all tests that have explicit 'rules' to be 'unsupported'?
There was a problem hiding this comment.
You mean like this test?
{"test_type": "collation_short", "label":"00003","s1":"9","s2":"\u0000","compare_type":"<1","test_description":" simple CEs & expansions","rules":"&\\x01<<<\\u0300&9<\\x00&\\uA00A\\uA00B=\\uA002&\\uA00A\\uA00B\\u00050005=\\uA003","hexhash":"d439f742bdde4b51012ee05519dfde424e25334a"}
Yeah, explicit "rules" are unsupported.
| } | ||
|
|
||
| if (result != true) { | ||
| if (result == true) { |
There was a problem hiding this comment.
Question about initializations of result and result_bool, even though they're not in this PR: shouldn't result and result_bool always be identical, by definition?
If so, can we just use one of them instead of both, when it comes to the code here in the body of this if statement?
There was a problem hiding this comment.
Yes, I'll remove result_bool.
| } | ||
|
|
||
| if (result != true) { | ||
| if (result == true) { |
There was a problem hiding this comment.
Leaving this comment here since I can't on lines above: aren't lines 43-45 redundant? We should be able to remove them b/c the logic is already covered by line 41, right?
These tests should be flagged as unsupported because the ECMAScript implementation does not support custom rules or other forms of comparison, i.e., levels of comparison.