Skip to content
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

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

Open
mxcl opened this issue Feb 10, 2016 · 34 comments
Open

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

mxcl opened this issue Feb 10, 2016 · 34 comments

Comments

@mxcl
Copy link
Contributor

mxcl commented Feb 10, 2016

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:

  • SR-1613 Require blocks runtime when compiling SourceKit on Linux
  • SR-1639 [SourceKit] Add in-process implementations of sourcekitd functions only defined for XPC

is blocked by:

duplicates:

  • SR-32 Find way to make allTests required / audited on OS X

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).

@modocache
Copy link
Mannequin

modocache mannequin commented Feb 10, 2016

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.

@modocache
Copy link
Mannequin

modocache mannequin commented Feb 10, 2016

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

@mxcl
Copy link
Contributor Author

mxcl commented Feb 10, 2016

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

@modocache
Copy link
Mannequin

modocache mannequin commented Feb 10, 2016

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
Copy link
Contributor Author

mxcl commented Feb 17, 2016

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.

@modocache
Copy link
Mannequin

modocache mannequin commented Feb 17, 2016

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

@ankitspd
Copy link
Member

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

@mxcl
Copy link
Contributor Author

mxcl commented Feb 19, 2016

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

@belkadan
Copy link

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
Copy link
Member

akyrtzi commented Feb 19, 2016

Note that SourceKit is not supported on linux.

@nkcsgexi
Copy link
Member

Would a code completion item help here?

@swift-ci
Copy link
Contributor

Comment by Honza Dvorsky (JIRA)

My attempt: #156

@modocache
Copy link
Mannequin

modocache mannequin commented Mar 29, 2016

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 #159 (comment)

  • Parsing Swift with an ad-hoc parsers is doomed to be perpetually buggy and become a maintenance liability.

  • Parsing -dump-ast is going to be fragile, or not even work at all, after we remove -dump-ast from production compilers to save code size.

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!

@ankitspd
Copy link
Member

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

@drewcrawford
Copy link
Contributor

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.
2. 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.

@modocache
Copy link
Mannequin

modocache mannequin commented Apr 2, 2016

> 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
Copy link
Contributor

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.

@modocache
Copy link
Mannequin

modocache mannequin commented Apr 2, 2016

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
Copy link
Contributor

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

@modocache
Copy link
Mannequin

modocache mannequin commented Apr 2, 2016

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
Copy link
Member

ddunbar commented Apr 3, 2016

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
Copy link
Contributor

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

@modocache
Copy link
Mannequin

modocache mannequin commented May 2, 2016

I made an attempt at adding `swiftc -frontend -dump-xctest-methods`: apple/swift#2364

Feedback appreciated!!

@modocache
Copy link
Mannequin

modocache mannequin commented May 10, 2016

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. apple/swift#2467 and apple/swift#2468 fix some initial CMake and build errors, with much more left undone.

@johnno1962
Copy link
Contributor

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

@modocache
Copy link
Mannequin

modocache mannequin commented May 22, 2016

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
Copy link
Collaborator

@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.

@modocache
Copy link
Mannequin

modocache mannequin commented May 25, 2016

@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).

@modocache
Copy link
Mannequin

modocache mannequin commented May 25, 2016

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!)

@modocache
Copy link
Mannequin

modocache mannequin commented May 29, 2016

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!

@modocache
Copy link
Mannequin

modocache mannequin commented May 31, 2016

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
Copy link
Collaborator

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

@modocache
Copy link
Mannequin

modocache mannequin commented Jun 3, 2016

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
Copy link
Contributor

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

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 4, 2022
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

No branches or pull requests