Skip to content

Unicode‐Heavy Test Fixture #2356

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.

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

Merged
merged 4 commits into from
Oct 2, 2019
Merged

Conversation

SDGGiesbrecht
Copy link
Contributor

@SDGGiesbrecht SDGGiesbrecht commented Oct 1, 2019

This idea originated on the forums here.

It puts all sorts of weird Unicode to use. The package manager handles all of it just fine at the moment. (On macOS at least; I guess CI will tell us how well Linux does.)

But it currently trips Xcode 11(.0) in at least three ways:

  1. The package name causes Xcode to crash when tests complete. (FB7328781)
  2. The dependency URL prevents Xcode from loading the manifest. (Paths don’t trigger this.)
  3. The nested package causes Xcode to crash while loading dependencies.

Likely Xcode isn’t the only client that can benefit from having this this fixture available to test itself against.


P.S. SwiftPM’s test manifests seem to out of date, as --generate-linuxmain tried to add other new tests. This PR only adds the test it creates to the test manifest, so as to avoid merge conflicts.

@jakepetroules
Copy link
Contributor

This is awesome! You could also throw in some long sequences of regional indicator symbols in there for good measure :)

@SDGGiesbrecht
Copy link
Contributor Author

@jakepetroules, is that a serious request or just humour? A single regional indicator is already in there (the UN flag) as the representative for multi‐scalar emoji, but I can string a few more on if you really want. (Such sequences bode trouble for slicing and wrapping, but are nothing special for equivalence, encoding or escaping, which are what matter to SwiftPM.)

P.S. The PRI (Public Review Issue) you linked was for Unicode 6.2.0 and it was never accepted in that form. (See the modifications log at the bottom of what was accepted.) Instead, no breaks are allowed between any two halves, and where line wrapping is desired in between flags, they can be grouped [half][half][ZWSP][half][half]. We are now at Unicode 12.1.0.

@SDGGiesbrecht SDGGiesbrecht changed the title Unicode‐Heavy Text Fixture Unicode‐Heavy Test Fixture Oct 1, 2019
@jakepetroules
Copy link
Contributor

Serious request, and thanks for the corrected link - I was trying to find one which succinctly described the original core issue, i.e. "Unfortunately, in a sequence of more than two of these characters, it is unclear where the segmentation boundaries lie".

"🇺🇸🇺🇸🇺🇸🇺🇸🇺🇸🇺🇸🇺🇸🇺🇸🇺🇸🇺🇸", for example, would have been interpreted as a single grapheme cluster (not 10) prior to changes to the grapheme break rules in Unicode 9 so it's another of those weird edge cases which could conceivably trip something up.

SwiftPM may use Swift.String but that's not guaranteed for clients like Xcode so it's a useful addition for something meant as an integration test, IMO.

@SDGGiesbrecht
Copy link
Contributor Author

I added a consecutive regional indicator. (The smiley face was why I wasn’t sure if the statement was serious.)

@aciidgh
Copy link
Contributor

aciidgh commented Oct 1, 2019

Thanks a lot, @SDGGiesbrecht!

@aciidgh
Copy link
Contributor

aciidgh commented Oct 1, 2019

@swift-ci smoke test

@SDGGiesbrecht
Copy link
Contributor Author

I don’t understand the CI result on Linux. I cannot even find the stuff I added in the output (I searched for the test name, test case name, directory name, etc. with ⌘F). Do you think the presence of weird file names is confounding CMake and Ninja, or does none of it have anything to do with this PR?

@aciidgh
Copy link
Contributor

aciidgh commented Oct 1, 2019

Seems like this failed in dispatch (which is built before swiftpm).

@aciidgh
Copy link
Contributor

aciidgh commented Oct 1, 2019

@swift-ci smoke test linux

@SDGGiesbrecht
Copy link
Contributor Author

Tests/FunctionalTests/MiscellaneousTests.swift:442: error: MiscellaneousTestCase.testUnicode : failed - Error Domain=NSCocoaErrorDomain Code=260 "The file doesn’t exist."

Should we #if !os(Linux) with a TODO for now so we can merge? I don’t have my Linux machine with me right now to debug it, but I can look into it more later when I do.

@aciidgh
Copy link
Contributor

aciidgh commented Oct 1, 2019

That's fine with me!

@SDGGiesbrecht
Copy link
Contributor Author

Done.

@aciidgh
Copy link
Contributor

aciidgh commented Oct 2, 2019

@swift-ci smoke test

@aciidgh aciidgh merged commit 172a0aa into swiftlang:master Oct 2, 2019
@SDGGiesbrecht SDGGiesbrecht deleted the unicode branch October 2, 2019 19:11
@SDGGiesbrecht SDGGiesbrecht mentioned this pull request Oct 5, 2019
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.

3 participants