Skip to content

Fix Unicode Tests on Windows #23040

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
Merged
merged 1 commit into from
Mar 11, 2019
Merged

Fix Unicode Tests on Windows #23040

merged 1 commit into from
Mar 11, 2019

Conversation

gmittert
Copy link
Contributor

@gmittert gmittert commented Mar 2, 2019

Whew did I learn far more than I could have ever wanted about Unicode.

S:\b\swift> C:\Python27\python.exe S:/b/llvm/bin/llvm-lit.py S:\b\swift\test-windows-x86_64\ -sv --filter ".*[Uu]nicode.*" -sv --show-xfail
...
********************                                                                                                          
Expected Failing Tests (1):                                                                                                   
    Swift(windows-x86_64) :: stdlib/UnicodeScalarDiagnostics.swift                                                            
                                                                                                                              
  Expected Passes    : 11                                                                                                     
  Expected Failures  : 1

Requires apple/swift-llvm#142 to go in to make the tests pass on Windows.

With python 2, lit still passes the arguments as utf-8 encoded bytes which is an issue on Windows. It wouldn't be too difficult of a fix, however, the gnu core-utils on Windows (ls being the one I ran into) don't understand UTF16 command line arguments and fail. To work around this, I've passed the arguments in response files. Alternatively, we'd either have to implement ls in lit, or (maybe more fun) in swift.

gmittert
gmittert commented Mar 2, 2019
View reviewed changes
test/stdlib/UnicodeMetadata.swift Outdated
@@ -1,4 +1,4 @@
// RUN: %target-build-swift -module-name="日本語01" %s -o %t.out
// RUN: %target-build-swift "@%/S/Inputs/Unicode.resp" %s -o %t.out
Copy link
Contributor Author

@gmittert gmittert Mar 2, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check // CHECK: [日本語01.myClass] in this file is checking against UTF-8 bytes. This prints out UTF-8 bytes on Windows, so that passes the check, but I wasn't entirely sure of the intended behaviour. Actually running it prints out a badly encoded mess.

S:\b\swift> &"s:\b\swift\test-windows-x86_64\stdlib\Output\UnicodeMetadata.swift.tmp.out.exe"
[日本語01.myClass]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, but probably correct. I think we assume our output streams are UTF-8ish now. @milseman?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't made any changes there. I believe it's been the case since before I started working on the project.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, just wanted to know if that was an assumption we were already making. Thanks!

@gmittert gmittert force-pushed the Use-The-Force-Luke-16 branch from e30ad9d to 2d6f1ff Compare March 2, 2019 04:32
@gmittert gmittert mentioned this pull request Mar 6, 2019
Merged
@gmittert gmittert force-pushed the Use-The-Force-Luke-16 branch from 2d6f1ff to 16dc909 Compare March 8, 2019 19:32
@gmittert
Copy link
Contributor Author

gmittert commented Mar 8, 2019

@jrose-apple @compnerd

jrose-apple
jrose-apple reviewed Mar 11, 2019
View reviewed changes
Copy link
Contributor

@jrose-apple jrose-apple left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this seems reasonable!

test/Driver/parseable_output_unicode.swift
// CHECK-NEXT: "command_arguments": [
// CHECK-NEXT: "{{.*[\\/]}}你好-[[OUTPUT]].o",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What changed here such that this is no longer the next line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It passes -target which comes first.

@jrose-apple
Copy link
Contributor

@swift-ci Please smoke test

@jrose-apple jrose-apple self-assigned this Mar 11, 2019
@jrose-apple jrose-apple merged commit d12e942 into swiftlang:master Mar 11, 2019
@gmittert gmittert mentioned this pull request Mar 11, 2019
Closed
@gmittert gmittert deleted the Use-The-Force-Luke-16 branch March 11, 2019 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@milseman milseman milseman left review comments

@jrose-apple jrose-apple jrose-apple approved these changes

Assignees

@jrose-apple jrose-apple

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gmittert @jrose-apple @milseman