search

swiftlang / swift-package-manager

The Package Manager for the Swift Programming Language
Apache License 2.0
9.71k stars 1.34k forks source link

[SR-710] Generate XCTestCaseProvider entries on Linux #5320

Closed mxcl closed 5 months ago

mxcl commented 8 years ago
Previous ID SR-710
Radar rdar://problem/32453385
Original Reporter @mxcl
Type Improvement

Attachment: Download

Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 6 | |Component/s | Package Manager, XCTest | |Labels | Improvement | |Assignee | None | |Priority | Medium | md5: d49557eb297404f7997ff4b6bf1a55f5

Sub-Tasks:

is blocked by:

duplicates:

Issue Description:

To test on Linux XCTestCase entries must conform to the protocol XCTestCaseProvider.

This means mostly mac developers probably won't bother making their tests run on Linux, which is not ideal.

XCTestCaseProvider will exist until Swift has reflection support on Linux, so we should try to generate these entries on Linux for now in the PM.

Use the AST to extract XCTestCase classes and their test functions, generate a source, add it to the test module.

Also generate an XCTMain for the full package suite.

This feature depends on me landing testing support (hopefully today).

ec882e32-f2b6-4d2a-849c-98d6c7df0cfb commented 8 years ago

Understanding that the initial testing support in the swift-package-manager is only intended to support swift-corelibs-xctest, this direction makes me feel a little uneasy.

A lot of the pain around third-party testing frameworks built on top of Xcode (https://github.com/kiwi-bdd/Kiwi/, https://github.com/specta/specta, https://github.com/Quick/Quick) originates from an assumption built into Xcode: that test methods are known at compile time. Because these frameworks dynamically generate test methods using custom functions and methods, Xcode does not support them. As a result, many of the Xcode testing navigator features do not work: no test indicators are displayed next to tests in the line number gutter, for example.

The key flaw in Xcode's testing support is that it does not provide a way for testing frameworks to indicate where tests are located. They cannot report test locations to Xcode, instead Xcode treats XCTestCase subclass methods that begin with "test" and take no arguments as tests, period. So these frameworks simply cannot support Xcode's test navigator features (outside of swizzling Xcode, which is not ideal).

I understand that providing a list of test methods automatically is crucial, and I'm totally on board with that in the short- or medium-term. But I do think this approach will have to be changed eventually in order to be more flexible, so I just thought I'd raise that point for future consideration.

ec882e32-f2b6-4d2a-849c-98d6c7df0cfb commented 8 years ago

Just to clarify: this is what I mean by " test indicators displayed next to tests in the line number gutter" above:

mxcl commented 8 years ago

Can you clarify your concerns? This work has no direct impact on Xcode.

ec882e32-f2b6-4d2a-849c-98d6c7df0cfb commented 8 years ago

Ah, sorry I wasn't clear. I used Xcode as an example of how assumptions that test functions will be known at compile time can make things difficult for non-XCTest testing tools. My only concern is that the approach you are proposing (using the AST to extract test functions) may make it harder for the Swift package manager to one day define a protocol that other test frameworks could work with.

mxcl commented 8 years ago

If it makes it harder then we have failed. I see this as strictly a solution for XCTest. Using XCTest should not come with caveats for some platforms.

ec882e32-f2b6-4d2a-849c-98d6c7df0cfb commented 8 years ago

Thanks for you response![]( Sounds excellent. Thanks for stewarding testing support, it's reassuring to see it in such capable hands)

aciidgh commented 8 years ago

should the generator use swiftc's -dump-ast mode to generate AST and then parse the AST to get the test methods names?

mxcl commented 8 years ago

@belkadan is there an AST variety that is considered stable?

belkadan commented 8 years ago

You could do something with SourceKit; talk to @akyrtzi or @nkcsgexi about that. Failing that, no, you would need to write your own tool that walks the AST and prints the information you need.

akyrtzi commented 8 years ago

Note that SourceKit is not supported on linux.

nkcsgexi commented 8 years ago

Would a code completion item help here?

swift-ci commented 8 years ago

Comment by Honza Dvorsky (JIRA)

My attempt: https://github.com/apple/swift-package-manager/pull/156

ec882e32-f2b6-4d2a-849c-98d6c7df0cfb commented 8 years ago

This is a major Swift 3 goal for both SwiftPM and corelibs-xctest, and I'd love to contribute in any way I can. I'm trying to understand the current state of things; here's the work that's been done so far:

However, both of these approaches have flaws, which @gribozavr sums up well in https://github.com/apple/swift-package-manager/pull/159#issuecomment-199993080:

Reflection would be a great tool here, but as @ddunbar mentions, it doesn't seem like those APIs are available in Swift yet. I've searched the mailing lists, but all I could find was this email that stated it was "one of our stretch goals for Swift 3".

@gribozavr and others in this thread have advocated instead to move the test scanning code from SourceKit into libIDE, and provide a documented and supported driver option in the Swift compiler to scan for tests. @ddunbar and others (myself included) have expressed misgivings in requiring test methods to be known at compile time, but we may need to punt on that discussion until after we achieve our Swift 3 goal of supporting XCTest.

SourceKit already has code to detect whether an instance method defines a test in XCTest: see SourceKit::FuncDeclEntityInfo.IsTestCandidate. So why not use SourceKit? SourceKit is meant to be an asynchronous wrapper for libIDE, and it implements that asynchronicity using XPC, which only works on OS X.

So if we move something like IsTestCandidate to libIDE, we can just use that, right? Well, not exactly, since that's a C++ library that we can't use directly from Swift (both SwiftPM and corelibs-xctest are written in Swift).

Seems to me there are several potential solutions here:

  1. Port SourceKit to Linux, using a different form of IPC on platforms where XPC is not available.

  2. Move business logic like IsTestCandidate to libIDE and add a C header (like libclang or sourcekitd). SwiftPM/XCTest will link against libIDE and use the C header.

  3. Move business logic like IsTestCandidate to libIDE and add a swiftc option to interface with its functionality. SwiftPM/XCTest will use swiftc.

I think #2 is the best option. It's less work than both #1 and #3. Logic like IsTestCandidate belongs in libIDE anyway--SourceKit should stick to XPC and asynchronous communication with libIDE.

Thoughts? Please comment on this issue if you have feedback or would be interested in taking this work on!

aciidgh commented 8 years ago

@modocache I can handle the swiftpm/xctest part of this

drewcrawford commented 8 years ago

I'm also invested in finding a solution here. Thanks to @modocache for an excellent summary.

Unfortunately, I have a concern completely out of left field. I would rather wish it away, but I'm afraid I've tried that, and it is still bothering me.

Existentialist crisis: what is a test? Pretend for a moment we have solved where to house IsTestCandidate. What is its definition? What does the function do?

For XCTest the answer is obvious: a test is a function that subclasses XCTestCase and starts with test. But is that the definition of a "test candidate"? It sounds more to me like an "*xc*test candidate". At best, we have commingled an internal implementation detail of XCTest with an internal implementation detail of libIDE, SourceKit, swiftc, or wherever this lands.

Meanwhile in another part of town, owensd (JIRA User) has been playing around with syntaxes for writing tests inline. Now, are they "tests" in the XCTest sense? Obviously not. But are they tests? I think they clearly are, and so if there is some compiler-level oracle that classifies functions as tests, it should classify these, in which case the oracle will no longer be useful for XCTest.

Now stepping way out into left field: does the problem begin and end with simply deciding if something is a test or not? e.g., what if I want to skip a test, or expect it to fail, or expect it to fail on Linux, or don't run it in parallel, or any number of other circumstances that actually arise in e.g. the Swift compiler. Now I have to create a bunch of metadata for each test, populate it in a separate file, so we know to skip it on Wednesdays or whatever the business logic requires.

But we are here because we have metadata in separate files now, and we think it is unmanageable. So right after we solve this we will back here figuring out where to house isParallelTestCandidate, isLinuxTestCandidate, etc.

My suggestion is that the interface we need is very different than isTestCandidate. The question of "what is a test?" has no universal answer and should be implemented inside XCTest, where the answer is clear and it is very appropriate to view the world through XCTest lens. And on the compiler side, we need tooling to allow XCTest to own its definition, so that it can evolve, change, experiment without compiler precoordination, and for that matter so can other ideas about test frameworks.

I believe we have a few options here:

1. A real machine-parseable source representation, as distinct from -dump-ast which is internal. Behind this door we would specifically craft a "file format" and consider it API. It could contain, initially, just the parts of the parse XCTest needs to find tests, but could be extended as needed to support other uses.

  1. An oracle that can answer arbitrary queries about Swift programs. Behind this door, XCTest would ask the oracle questions like: "Find me all subclasses of XCTestCase." "Find instance methods of class Foo that start with test". And based on this information XCTest will classify things as tests or not. In this scenario we have another choice: whether our oracle should be static (e.g. in libIDE or similar) or dynamic (e.g. in the runtime / reflection). Figuring out where to store metadata in the dynamic case is a bit sticky, but it may be possible. But in any case the distinction is that, while current libIDE proposals assume a single API call, I think the surface area is larger.

In summary, I really appreciate all the energy and effort going into solving this. My concern is making sure we actually do solve it, so we are not back here again a few months from now.

ec882e32-f2b6-4d2a-849c-98d6c7df0cfb commented 8 years ago

> It sounds more to me like an "*xc*test candidate".

Yes, I believe I raised these concerns as well--see the first comment from February. However, the Swift 3 goal (and the only thing that's been agreed upon via the evolution process) for SwiftPM is integration with XCTest. I think it's important to treat this task as simply a method to generate a list of XCTest tests. I'm of the opinion, therefore, that the list should be generated by swift-corelibs-xctest itself. However, I'd also like to leave this discussion until after we have a working implementation.

> what if I want to skip a test, or expect it to fail, or expect it to fail on Linux, or don't run it in parallel, or any number of other circumstances

XCTest doesn't support any of these features. Let's flesh this out on swift-evolution.

drewcrawford commented 8 years ago

I'm missing something. If we are only worried about XCTest here, why not _functionsByClass("module.Class")? We already have _typeByName("module.Class") in the runtime, so this seems like an obvious extension of an existing design.

ec882e32-f2b6-4d2a-849c-98d6c7df0cfb commented 8 years ago

The original task description includes the following:

> XCTestCaseProvider will exist until Swift has reflection support on Linux

My bad--I neglected to check up on the status of reflection before commenting with my understanding of the "current state of things" above. Is reflection support on Linux almost ready? I would definitely in favor of simply using reflection if it's available.

drewcrawford commented 8 years ago

IIRC, _typeByName is ready. If _functionByClass is our only barrier, I would consider PRing it myself if that would move testing forward.

ec882e32-f2b6-4d2a-849c-98d6c7df0cfb commented 8 years ago

Definitely, that would be a huge help! The default implementation of XCTestCase.allTests should use that reflection to return a list of methods with a return type of Void, and which begin with test.

ddunbar commented 8 years ago

I'm not privy to current discussions on function reflection, but I suspect getting all of the reflection pieces we need for this designed and in place is an involved project. Does anyone know if these discussions have already happened on swift-evolution (links?), I'm afraid I don't follow it closely enough to know.

johnno1962 commented 8 years ago

While we wait for a properly designed function reflection, there’s an improperly designed way you can use runtime Metadata that does exist to call tests: https://github.com/johnno1962/TestRunner

ec882e32-f2b6-4d2a-849c-98d6c7df0cfb commented 8 years ago

I made an attempt at adding `swiftc -frontend -dump-xctest-methods`: https://github.com/apple/swift/pull/2364

Feedback appreciated!!

ec882e32-f2b6-4d2a-849c-98d6c7df0cfb commented 8 years ago

For those not following the mailing list, we've decided to use SourceKit to implement this feature: https://lists.swift.org/pipermail/swift-dev/Week-of-Mon-20160502/001940.html

This means we need to port SourceKit to Linux. https://github.com/apple/swift/pull/2467 and https://github.com/apple/swift/pull/2468 fix some initial CMake and build errors, with much more left undone.

johnno1962 commented 8 years ago

There is a fork with a port to Linux of sorts here:
https://github.com/johnno1962/swift/commits/swift-2.2-format5

ec882e32-f2b6-4d2a-849c-98d6c7df0cfb commented 8 years ago

I don't mean to "claim ownership" over this task, but I am working on it (albeit during my free time). If someone else wants to claim it, please feel free! 🙂

briancroom commented 8 years ago

@modocache I am definitely also interested in contributing toward this, but I'm a little bit uncertain where it would be most effective to start looking without wanting to get in the way of the good work you've been doing. I have pretty limited time to spend on this, but the task feels big enough that I'm hoping we can parallelize some tasks here. Do you think you would be in a position to create some additional Jira tasks/issues that relate to the remaining work? That would help with coordinating this as the Swift 3 release inches closer.

ec882e32-f2b6-4d2a-849c-98d6c7df0cfb commented 8 years ago

@briancroom: Ha, we are definitely on the same wavelength! I noticed your comment just after creating sub-task https://bugs.swift.org/browse/SR-1613. I'll try and split this up into granular tasks that we can divvy up amongst ourselves (and anyone else who wants to work on this).

ec882e32-f2b6-4d2a-849c-98d6c7df0cfb commented 8 years ago

I think SourceKit has three "dependencies" that we need to think about when porting to Linux:

  1. Blocks (already covered in https://bugs.swift.org/browse/SR-1613)

  2. Grand central dispatch, which I think is used in a few places. We might be able to use swift-corelibs-dispatch on Linux.

  3. XPC. From what I understand SourceKit has "pluggable" backends, so we could theoretically swap out XPC for an in-process model. However, testing tools like sourcekitd-test make heavy use of XPC, and we need to test SourceKit on Linux.

Besides working on https://bugs.swift.org/browse/SR-1613, creating tasks for these items and getting feedback on them would also be a great way to help. (Or pointing out my flawed reasoning!)

ec882e32-f2b6-4d2a-849c-98d6c7df0cfb commented 8 years ago

I added sub-task https://bugs.swift.org/browse/SR-1639. As far as I can tell, we're missing several function definitions for running in-process SourceKit. Chime in on that issue if you have any ideas here, or would like to help!

ec882e32-f2b6-4d2a-849c-98d6c7df0cfb commented 8 years ago

For those watching this issue: I could use some help here. I have a few pull requests out there for porting SourceKit to Linux, but I need some feedback from the SourceKit maintainers – it's hard to continue working on this without some indication that I'm moving in the right direction.

Here are the pull requests:

I'm not asking that these pull requests get merged, necessarily. I'd simply like to know if I'm spinning my wheels for nothing over here. Especially important to me is the question of whether to remove SourceKit's libdispatch dependency on Linux. That's a lot of work, and I don't want to spend a ton of time on it, just to find out it won't be merged.

Thanks! 🙂

briancroom commented 8 years ago

Note that I've just opened SR-1676 to more specifically track the (substantial) work of getting SourceKit up and running on Linux.

ec882e32-f2b6-4d2a-849c-98d6c7df0cfb commented 8 years ago

Awesome, thanks Brian! I'm probably going to take a break from this one for a while. It's getting warmer in NYC and I want to spend some more time in the sun 🙂

I'd still love feedback on my outstanding pull requests and will try to get them merged, of course!

abertelrud commented 8 years ago

While highly desirable, this seems out-of-scope for Swift 3 at this point.

MaxDesiatov commented 5 months ago

IIUC this was resolved for some time now with test discovery available for XCTest via index store on non-Darwin platforms.