Skip to content

Conversation

@modocache
Copy link
Contributor

Enhance xctest_checker such that it can parse "CHECK" prefixes that are in the middle of a line (instead of just at the very beginning).

In addition, use a common XCTestCheckerError to ensure all functional test suite failures are displayed inline in Xcode.


This splits the first part of #68 out into its own pull request. It also only changes one functional test, in order to minimize merge conflicts.

Enhance xctest_checker such that it can parse "CHECK" prefixes that
are in the middle of a line (instead of just at the very beginning).

In addition, use a common `XCTestCheckerError` to ensure all functional
test suite failures are displayed inline in Xcode.
@briancroom
Copy link
Contributor

Very nice! This looks like a great enhancement.

One thing I wonder about though - since we are moving towards expecting the presence of a complete Swift development environment (#77), might it make sense to reevaluate the decision to not use FileCheck itself, rather than maintaining our own tool and gradually re-implementing additional bits of FileCheck behavior? For easier reference, most of the original discussion around this can be found at #14.

@modocache
Copy link
Contributor Author

Yeah, that makes sense. It would mean much less code to maintain in this repository. @mike-ferris-apple, what do you think? In a nutshell, this would mean one could not test corelibs-xctest without having built LLVM first.

One downside is that FileCheck wouldn't provide the same Xcode failure highlighting--I've grown to really appreciate that! Maybe we can contribute that to the LLVM project...

@ddunbar
Copy link
Contributor

ddunbar commented Apr 9, 2016

Please do consider contributing upstream whatever is necessary to make that work (I didn't look at the original change very closely).

We could probably get FileCheck embedded in the toolchains if we wanted to be able to use them to run tests, certainly for dev snapshots that would make sense, and I think is small enough it doesn't impact the download significantly even for people that don't need it.

@mike-ferris
Copy link

Having more standardization would be good. I like @ddunbar 's idea of getting FileCheck included in the toolchains as well. It would be nice to not necessarily have to build all of llvm just for this.

@modocache
Copy link
Contributor Author

I gave this a go, and noticed one complication: we're talking about the built FileCheck executable here, which means it will be in a directory like Ninja-ReleaseAssert/llvm-macosx-x86_64/bin/FileCheck, or Makefiles-Debug/llvm-linux-x86_64/bin/FileCheck -- the point is, we won't be able to pick a sensible default.

On Linux, this is no problem: we encourage people to build using the Swift build script, which knows the build mode. It will be able to find FileCheck no matter the build configuration used for LLVM.

For OS X users building via Xcode, however, we're out of luck. We can't guess where their FileCheck executable is. We'd need users to specify the path to the executable for us, which I think would baffle most first-time contributors.

Thanks to the .xctoolchain suggestions, I can see one way forward here: we copy llvm-lit, FileCheck, and not from the LLVM bin dir into the .xctoolchain. Our Xcode build would then use these.

I think that would take a bit of work, though. And assuming we'll be using xctest_checker in the interim, I think it's worth improving it where we can.

Let me know if I'm missing something here. If not, give the code in this pull request a review (and kick off the CI tests if you can!).

@mike-ferris
Copy link

@swift-ci please test

@modocache modocache merged commit 440a092 into swiftlang:master Apr 11, 2016
@modocache modocache deleted the xctest-checker-inline branch April 11, 2016 17:50
@modocache
Copy link
Contributor Author

Merged! But we should (at some point soon) reach a consensus on whether to include internal LLVM tools like FileCheck and llvm-lit in .xctoolchains.

@ddunbar
Copy link
Contributor

ddunbar commented Apr 28, 2016

We reached a consensus on including FileCheck in the toolchain, I am working on a patch but testing the install process is painful and I am highly distracted. lit can be installed via pip in --user mode to have a zero-install build/test pipeline (llbuild's xcodeproj does this automatically, for example).

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.

4 participants