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

Add test for Test module. #143

Closed
wants to merge 3 commits into from

Conversation

kostiakoval
Copy link
Collaborator

Check that Package.xctest are generated.

func testModuleWithTests() {
fixture(name: "SwiftTesting/SingleTarget", file: #file, line: #line) { prefix in
XCTAssertBuilds(prefix)
print(prefix)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should print be there in tests?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks. Fixed

@modocache
Copy link
Collaborator

Awesome! Would love to see this and more tests for the test module merged.

@kostiakoval
Copy link
Collaborator Author

Me to :) Not sure if this test, and other, related to tests module, should be in a separate file?

@mxcl
Copy link
Contributor

mxcl commented Feb 23, 2016

On Linux it has a different name. Also needs the other Linux machinery.

@kostiakoval
Copy link
Collaborator Author

Like that? I haven't tested it on linux, hope it will work :)

@mxcl
Copy link
Contributor

mxcl commented Feb 24, 2016

Not quite, there is only one LinuxMain.swift.

@kostiakoval
Copy link
Collaborator Author

LinuxMain.swift is for a Fixture, so I can run test manually on the Linux.
I've update it to latest testing syntax and tested on Linux, it works.

Is there anything I should with it?

@ankitspd
Copy link
Member

ankitspd commented Mar 2, 2016

👌

XCTAssertDirectoryExists(prefix, ".build", "debug", "Package.xctest")
#else
//FIXME: test-Package not generated during swift-build on linux so this will fail.
//XCTAssertFileExists(prefix, ".build", "debug", "test-Package")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should work, I'm confused, something doesn't work?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

XCTAssertBuilds calls executeSwiftBuild which runs swift build,
test-Package executable is generated when "test" target is built

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course. Well, we should run swift test.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I will fix it. Thanks for feedback

@kostiakoval
Copy link
Collaborator Author

@mxcl I've added XCTAssertTests to run swift-test. Works both on Mac and Linux.

@mxcl
Copy link
Contributor

mxcl commented Mar 9, 2016

@swift-ci Please test

@kostiakoval
Copy link
Collaborator Author

@modocache Can I get any help why tests are failing, have any ideas or tips?

tmp/../SingleTarget/Tests/Foo/Test.swift:2:8: error: no such module 'XCTest'
import XCTest

@mxcl
Copy link
Contributor

mxcl commented Mar 9, 2016

@kostiakoval platform? On OS X don't build swift with xctest, on Linux you also have to build foundation now and depending on how you invoke the tests, tell the system where to find it.

@kostiakoval
Copy link
Collaborator Author

I'm not sure how to pass those parameters. From what I see we can't pass any arguments to swift-test on linux. Should I send some environment variables?

I would appreciate any help please .

@modocache
Copy link
Collaborator

Thanks again for working on this, @kostiakoval! I think it'll be really cool to not only test corelibs-xctest's behavior (which we do in corelibs-xctest), but also the behavior of the swift test command.

First off as a basic sanity check I pulled down the latest revision on master for all repositories onto my Ubuntu 15.10 machine and ran:

$ utils/build-script -R --swiftpm --xctest --test -- \
    --skip-test-cmark --skip-test-swift --skip-test-foundation

This succeeded. I then tried to pull in the changes from your pull request locally:

$ cd ../swiftpm
$ git pull https://github.com/kostiakoval/swift-package-manager.git TestTarget-tests

Re-running the Swift utils/build-script from before, I get the following build error:

Compiling Swift Module 'Gettest' (4 sources)
/home/modocache/GitHub/apple/swiftpm/Tests/Get/Utilities.swift:136:20: error: use of unresolved identifier 'Resources'
    let toolPath = Resources.findExecutable("swift-test")
                   ^~~~~~~~~
/home/modocache/GitHub/apple/swiftpm/Tests/Transmute/Utilities.swift:136:20: error: use of unresolved identifier 'Resources'
    let toolPath = Resources.findExecutable("swift-test")
                   ^~~~~~~~~
/home/modocache/GitHub/apple/swiftpm/Tests/Functional/Utilities.swift:136:20: error: use of unresolved identifier 'Resources'
    let toolPath = Resources.findExecutable("swift-test")
                   ^~~~~~~~~
/home/modocache/GitHub/apple/swiftpm/Tests/ManifestParser/Utilities.swift:136:20: error: use of unresolved identifier 'Resources'
    let toolPath = Resources.findExecutable("swift-test")
                   ^~~~~~~~~
<unknown>:0: error: build had 4 command failures
error: exit(1): /home/modocache/GitHub/apple/build/Ninja-ReleaseAssert/llbuild-linux-x86_64/bin/swift-build-tool -f /home/modocache/GitHub/apple/build/Ninja-ReleaseAssert/swiftpm-linux-x86_64/debug.yaml test
bootstrap: error: tests failed with exit status 1
utils/build-script: command terminated with a non-zero exit status 1, aborting

I seem to recall that Resources was renamed or removed in a recent commit (I think it may have been Xcodeproj support), so your commit probably needs to be updated to reflect that.

Anyway, silly me, I guess I probably should have tested exactly your changes, and not them merged into master! 😅 I tried just that:

$ git remote add kostiakoval https://github.com/kostiakoval/swift-package-manager.git
$ git fetch kostiakoval 
$ git checkout TestTarget-tests

But unfortunately this time I get Swift 3-related errors.

I'll try to checkout pre-Swift 3 versions of all the Swift libraries and re-run your pull request. In the meantime, if you could rebase your changes onto the latest swiftpm master branch and fix up the build errors, that'd be super helpful! 🙇

@modocache
Copy link
Collaborator

Because of the above I'm not able to test this out myself, but my current theory is that you'll need to pass flags to executeSwiftTest(), as the test utilities in SwiftPM already do for executeSwiftBuild() and XCTAssertBuilds().

In the ModuleMapsTestCase, for example, I see that -L /some/output/directory is passed to XCTAssertBuilds(). You'll probably need to do the same with executeSwiftTest(), but passing some subset of the flags I pass here.

A corelibs-foundation framework is included with Swift snapshot releases. If you're interested in having the tests use the currently installed version, then you can link that one. But I imagine that in order for this test to be robust, you'd want to allow a path to a Foundation framework to be specified somehow...

@kostiakoval kostiakoval force-pushed the TestTarget-tests branch 2 times, most recently from b813132 to 82a562f Compare March 11, 2016 08:28
@kostiakoval
Copy link
Collaborator Author

@modocache Thanks for you amazing feedback!
I've fixed the Resources.findExecutable("swift-test") build error. Tests are working again on MacOS.

@mxcl Can we run CI to see if we still have that error on Linux?

@mxcl
Copy link
Contributor

mxcl commented Mar 11, 2016

@swift-ci Please test

@kostiakoval
Copy link
Collaborator Author

When I try to run test on Linux, even from the master repo, I get this error

./Utilities/bootstrap test

/home/kkoval/apple/swiftpm/Tests/PackageType/PackageNameTests.swift:12:8: error: no such module 'XCTest'
import XCTest
...

@mxcl
Copy link
Contributor

mxcl commented Mar 11, 2016

On linux you have to specify the path to: --xctest

See Utilities/bootstrap --help.

@modocache
Copy link
Collaborator

On linux you have to specify the path to: --xctest

You now must also specify the path to Foundation, using --foundation. I've been meaning to add this to the README. Those instructions are also in Utilities/bootstrap --help.

@mxcl
Copy link
Contributor

mxcl commented Mar 11, 2016

Honestly it is better to run the tests via the swift build-script. I use:

swift/utils/build-script \
    --swiftpm --llbuild --xctest --foundation --test --release -- \
    --skip-test-swift --skip-test-cmark --skip-test-xctest --skip-test-foundation

Though obviously this builds the whole of swift. So…

@kostiakoval
Copy link
Collaborator Author

Yes that's right. But how can I specify --xctest path when I call swift-test.
Right now when we call swift-test on Linux we don't pass any arguments to swift-corelibs-xctest.
https://github.com/apple/swift-package-manager/blob/master/Sources/swift-test/test.swift#L26

Does swift-corelibs-xctest support it now?

@ankitspd
Copy link
Member

@kostiakoval I was able to fix by just changing this ankitspd@772335d
I built toolchain on my Linux VM with

$ ./swift/utils/build-script --preset="buildbot_linux" install_destdir=/home/aciid/hdd/toolchain/swift-nightly installable_package=/home/aciid/hdd/toolchain/swift.tar.gz

@mxcl
Copy link
Contributor

mxcl commented Mar 13, 2016

But how can I specify --xctest path when I call swift-test.

You can't, this is not a supported execution model. swift-test must be part of a complete toolchain or it must use a complete toolchain via the TOOLCHAINS environment variable.

@kostiakoval
Copy link
Collaborator Author

The problem was in wrong naming. I've tested it local and it works.
@mxcl Can we invoke CI and get it merged :)

@mxcl
Copy link
Contributor

mxcl commented Mar 22, 2016

Conflicts.

@kostiakoval
Copy link
Collaborator Author

@mxcl Fixed. And cleaned up commits

@kostiakoval
Copy link
Collaborator Author

@mxcl Could you please invoke "CI test" and merge it please? Also PR #122 is ready

@ankitspd
Copy link
Member

👍

@mxcl
Copy link
Contributor

mxcl commented Mar 31, 2016

@swift-ci Please test

@czechboy0
Copy link
Contributor

@kostiakoval Tests failed on Linux, can you please take a 👀 ?

@kostiakoval
Copy link
Collaborator Author

I found how to reproduce that bug. It works fine if you run ./Utils/bootstrap test with no extra parameters.
I happens when you run it and provide --swiftc --xctext --foundation --sbt options.
Now I need to find a way to fix it :)

@mxcl
Copy link
Contributor

mxcl commented Apr 4, 2016

I've tried to make this work on Linux and my conclusion is we shouldn't bother with this test:

  1. It is hard to make this work in the context of a full build of swift since the paths for XCTest and Foundation are not in the standard "toolchain" shape.
  2. We are always testing swift-test because we use swift-test to test ourselves.

Instead we could add an integration test, but really we don't even need to because we test swift-test when we test ourselves.

So I'd like to close this, if there are no objections.

@kostiakoval
Copy link
Collaborator Author

I agree with point 1. I've opened that PR because some Tests folder layout didn't work
#162

I agree to close that PR and find a better way to test it.

@czechboy0
Copy link
Contributor

@mxcl If we can find a way to still implement something like XCTAssertBuilds but for tests, I'm fine with closing this one. But I definitely think we need to validate layouts not just with swift-build, but also with swift-test, because completely valid layouts have (twice already) been broken on a released snapshot. I believe we could have prevented the second breakage if we had XCTAssertTests, because in that case swift-build worked but swift-test failed to compile tests. So I'm only OK with closing this PR if there's a strategy you have in mind that can give us the ability to validate a layout for swift-test. I'm afraid that if we don't start testing out layouts completely, sooner or later we'll break swift-test again for projects in the wild.

@kostiakoval Exactly why I'm following this PR - I had my projects break on a snapshot bc swift-test didn't compile test on a layout that was only being tested by running swift-build.

@mxcl
Copy link
Contributor

mxcl commented Apr 4, 2016

This PR doesn't test layouts. But I agree we should test layouts. We don't have to do it functionally. Would be better to have unit tests for that component.

@mxcl mxcl closed this Apr 4, 2016
ankitspd pushed a commit to ankitspd/swift-package-manager that referenced this pull request Jan 11, 2019
…endency-cycles

Make cycle information available to the client
MaxDesiatov pushed a commit to MaxDesiatov/swift-package-manager that referenced this pull request Jul 6, 2020
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.

None yet

5 participants