Fixes SR-822, support tests directly under Tests directory

[bhargavg]

Added support for test files directly under Tests directory.

Sample:

Foo
|
- Package.swift
|
- Sources
|   - Bar.swift
- Tests
    - BarTests.swift

[kostiakoval]

Add Linux support. It should be like

#if os(Linux)
extension FooTests: XCTestCaseProvider {
    var allTests: [(String, () throws -> Void)] {
        return [
            ("testSuccess", testSuccess),
        ]
    }
}
#endif

[kostiakoval]

PR-158 is merged now, please update your tests.

[bhargavg]

@aciidb0mb3r @kostiakoval Made the changes and rebased. Please check.

[aciidgh]

I think this might look better with guard

guard !(testDirectories.count > 0 && rootTestFiles.count > 0) else { throw ModuleError.InvalidLayout(.InvalidLayout) } 

if testDirectories.count > 0 {
    return try testDirectories.map { 
        TestModule(basename: $0.basename, sources: try self.sourcify($0)) 
    }
}
if rootTestFiles.count > 0 {
     let rootTestSource = Sources(paths: rootTestFiles, root: path)
     return [TestModule(basename: name, sources: rootTestSource)]
}
return []

[bhargavg]

👍 Done

[bhargavg]

Could someone please trigger CI tests on this PR?

[mxcl]

@swift-ci please test

[bhargavg]

Fixed issue that caused build to fail.
@mxcl Could you please re-trigger CI?

[kostiakoval]

I'm not sure I 100% agree with that implementation. Why can't we support both tests files in root Tests folder and subfolders with tests at the same time?

Just by simple adding 1 test file in the root, you can break your project. I think that could be confusing for people.

We could simple support both of them by implementing it like that.

extension Package {
    func testModules() throws -> [TestModule] {
        let (directories, files) = walk(path, "Tests", recursively: false).partition{ $0.isDirectory }

        let testDirectories = directories.filter{ !excludes.contains($0) }
        let rootTestFiles = files.filter { 
            !$0.hasSuffix("LinuxMain.swift") && isValidSource($0) && !excludes.contains($0)
        }
        var res: [TestModule] = []

        if (testDirectories.count > 0) {
            res + try testDirectories.map {
                TestModule(basename: $0.basename, sources: try self.sourcify($0)) 
            }
        }
        if (rootTestFiles.count > 0) {
            let rootTestSource = Sources(paths: rootTestFiles, root: path)
            res.append(TestModule(basename: name, sources: rootTestSource))
        }

        return res
    }
}

What do you think?

[bhargavg]

Yes @kostiakoval, I thought about the same. In fact, in swift-build-dev mailing list, I've asked whether we could support the following layouts.

layout-1
├── Package.swift
└── Sources
    └── ModuleA
        ├── Bar.swift
        ├── Foo.swift
        └── Tests
            └── ModuleATests.swift

Current Status: Building - Tests Failing


layout-2
├── Foo.swift
├── Package.swift
└── Tests
    └── FooTests.swift

Current Status: Building - Tests Failing


layout-3
├── Package.swift
├── Sources
│   ├── Foo.swift
│   └── ModuleA
│       ├── Bar.swift
│       └── Tests
│           └── BarTests.swift
└── Tests
    └── FooTests.swift

Current Status: Build Failing - Invalid Layout


layout-4
├── Package.swift
├── Sources
│   ├── Foo.swift
│   ├── ModuleA
│   │   └── Bar.swift
│   └── Tests
│       └── FooTests.swift
└── Tests
    └── ModuleA
        └── BarTests.swift

Current Status: Build Failing - Invalid Layout

Max Howell, told that he is drafting a proposal that will make folder layouts to be more flexible. That's why I haven't added support for tests directly under Tests directory along with sub directories.

So, what do you think? Shall we wait for the proposal to come out? or can we merge this one and refactor once the proposal is out?

[kostiakoval]

Yes, I've seen the mail, and If I understood Max correctly

called “No More Magic”

we should include all test that are in a Root folder for tests (the Tests folder)
I think @mxcl is the correct person to answer that question.

[mxcl]

Of course, we cannot actually test a single module executable, so we should probably address that separately.

[kostiakoval]

This is still a valid issue. Can we resurrect work on it? @bhargavg @mxcl

swift test produce very bad error if I try to run it with test in Tests folder

fatal error: unexpectedly found nil while unwrapping an Optional value: file   
apple/swift/stdlib/public/core/Optional.swift, line 136

[bhargavg]

@kostiakoval I'm still waiting for "No More Magic" proposal to come out which should draw the line between supported and unsupported layouts.

For now, as you might've already guessed, you can create folder inside Tests with module name to prevent this error.

[kostiakoval]

@bhargavg I think it would be better to throw an exception with diagnostics info that this Test folder layout is not supported for now and give tips to users how to fix it.
It's way better than having fatal error:

@mxcl any thoughts?

[bhargavg]

@kostiakoval Agreed. Raised PR #301 making swift test to throw an error instead of failing with a cryptic error message.

[mxcl]

I'm torn between:

1. Insisting Tests/ have subdirs
2. Being consistent with Sources/

So maybe someone else can weigh in.

I like 1. as things are then less ambiguous in how our code works and what the user will get when experimenting. But the reality is we already support Sources/ without subdirs.

[ddunbar]

My vote is consistency with Sources, I am surprised when it isn't.

[ddunbar]

I'm closing this due to age, I think we are going to probably end up going in the direction of not accepting this to enforce consistency with a more scalable convention (subdirectories). In any case, we should resolve it in a proposal or bug, not decide in this PR.