[test] fix for hasPrefix and hasSuffix test

[glessard]

The previous version wrongly cached a result, which in turn masked a bug in the bridged hasSuffix().
(reported as https://bugs.swift.org/browse/SR-243)

This version also trips hasPrefix, but that involves \u{0}; the correct behaviour there is less obvious.

[glessard]

This bug fix surfaces two cases that always failed, but were masked.
They are now both annotated with xfail and links to bug reports.

[glessard]

Rebased after the changes in #1131

[tkremenek]

@swift-ci test

[tkremenek]

@gribozavr please review

[glessard]

@tkremenek The swift-ci failure was unrelated to these changes; test again?

[jrose-apple]

Drive-by test restarting:

@swift-ci Please test OS X platform

[jckarter]

@gribozavr This ok to merge?

[gribozavr]

@glessard Sorry, I don't understand what this patch changes -- it looks like a pure refactoring to me. I see that you are removing the xfail predicates from the test list, but you're adding them back with the map() function. Why is this making a difference? I'm probably missing something, I would be glad if you could explain to me.

[glessard]

(edited)
The bug fix is in the first commit, the rest is dealing with the consequences of it (and of #1131, since they have distinct sets of xfails).

In the original version of 'checkHasPrefixHasSuffix', a pair of expected results is calculated, and then are used twice each. This wasn't correct. In particular, if the first character of s is combining, s.hasSuffix(s) is true, but ("abc"+s).hasSuffix(s) is false. However, the test as originally written expected both answers to be the same (see b6bc624#diff-f7824440e4e317d38fc0554573876bccL256). The original test passed the ("\u{0301}", "\u{0341}") case because of a bug in NSString/CFString (https://bugs.swift.org/browse/SR-243). Without that NSString bug, the original formulation of the test wouldn't have passed.

The fix simply ensures the expected result is recalculated for every single use, by moving the extra suffix and prefix tests to a separate call of checkHasPrefixHasSuffix. (I stumbled upon this while porting the tests for #440)

[glessard]

@gribozavr Did the above response clarify the patch?

[CodaFi]

It doesn't seem like anything else in the file is using tests. Why not just fold the results of the map into the array itself?

[glessard]

The tests for comparisons and substrings (prefix and suffix) don't overlap cleanly, so I separated them. The array is also mapped in the commit that follows this one (line 281).

[CodaFi]

There definitely needs to be a little bit of documentation on both of the maps and on the special cases of the literal here explaining just that. It's hard to see where and why things XFail without it.

[CodaFi]

@swift-ci please smoke test.

[CodaFi]

Now that this is just a change to a default argument, this initializer isn't needed.

[glessard]

Agreed.

[CodaFi]

I think I'm out of nitpicks if this was pure refactoring. Thank you @glessard.

[CodaFi]

Once you've added a bit of docs, please squash this down to a patch or two.

[glessard]

@CodaFi I think this should do it; thanks for the feedback

[CodaFi]

@swift-ci please smoke test.

[CodaFi]

macOS
Linux

// @shahmishal

[shahmishal]

This issue looks like:
janinko/ghprb#150
janinko/ghprb#284

going to retrigger the build to see if it picks up the new sha.

@swift-ci Please smoke test

[CodaFi]

I think we're good to go here. Merging.

[jrose-apple]

Ah, @gribozavr, did you have any remaining comments?

[gribozavr]

LGTM, thanks!