-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Fix 2 Driver tests on Windows #20172
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
Conversation
d072e1d
to
4ee5438
Compare
Thanks for looking at this. I don't think this is necessarily the right change for actions.swift, which is already XFAILed on Linux and FreeBSD. I'd rather split it up into the target-agnostic tests and the target-specific tests—and then yes, a DEBUG_NONDARWIN (or DEBUG_NON_DSYM) check would make sense. |
So if I'm understanding correctly, leave actions.swift as is, but also xfail on windows, then add another test which doesn't check for dsym and doesn't xfail? I mostly just fixed these so I'd have a passing test on Windows to add in a test for #20140, but if I were to go through and look at a few more, is one large PR easier, or is several each with a handful of tests easier? |
83acc10
to
f29b3ff
Compare
Updated to xfail on windows and split the actions test into dsym and non dsym. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the slow feedback!
test/Driver/actions.swift
Outdated
@@ -165,5 +158,5 @@ | |||
// WHOLE-MODULE: 3: compile, {0, 1, 2}, object | |||
// WHOLE-MODULE: 4: link, {3}, image | |||
|
|||
// RUN: %swiftc_driver -driver-print-actions -g %S/Inputs/main.swift %S/../Inputs/empty.swift %s -module-name actions -force-single-frontend-invocation 2>&1 | %FileCheck %s -check-prefix=WHOLE-MODULE -check-prefix=WHOLE-MODULE-DEBUG | |||
// RUN: %swiftc_driver -driver-print-actions -g %S/Inputs/main.swift %S/../Inputs/empty.swift %s -module-name actions -force-single-frontend-invocation 2>&1 | %FileCheck %s -check-prefix=WHOLE-MODULE | |||
// WHOLE-MODULE-DEBUG: 5: generate-dSYM, {4}, dSYM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're no longer using this FileCheck line, so you can just remove it. Actually, the invocation isn't really adding anything either at this point.
test/Driver/actions-dsym.swift
Outdated
|
||
// XFAIL: freebsd, linux, win32 | ||
|
||
// RUN: %swiftc_driver -driver-print-actions %s 2>&1 | %FileCheck %s -check-prefix=BASIC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I don't think this file should include any of the actions that don't use -g, since the tests are the same. And then I think it's okay for this file to use an explicit -target x86_64-apple-macosx10.9
like you had originally, so that we can remove the XFAILs. That's a compromise, but I think it's a reasonable one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me, I've removed them.
f29b3ff
to
06f6f4a
Compare
@swift-ci Please test |
This comment has been minimized.
This comment has been minimized.
Build failed |
Looks like there's a -target missing somewhere. |
Oof yeah, that should be removed anyways since it's not being passed -g |
06f6f4a
to
5dadb73
Compare
Most of the driver tests currently don't work on Windows, but these two were relatively fixable. - Moved the dsym checks to actions-dsym and ignore that on windows - alternatively match the full path of dsymutil, which on Windows is given as "C:\....\dsymutil.exe" verify-debug-info ...
5dadb73
to
84cb56b
Compare
Alright, updated and tested locally against all of Windows, MacOS, and Linux. |
@swift-ci Please test |
Build failed |
Build failed |
Thanks, sorry for all the delay! |
Most of the driver tests currently don't work on Windows, but these two
were relatively fixable.
given as
"C:\....\dsymutil.exe" verify-debug-info ...
Would it be useful to have a DEBUG_NONDARWIN etc that is the same as the DEBUG check but doesn't check for generate-dSYM and doesn't pass
-target
?