Skip to content

Rename xctest executable to avoid inferring it as module name #282

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

Merged
merged 1 commit into from
Apr 28, 2016

Conversation

aciidgh
Copy link
Contributor

@aciidgh aciidgh commented Apr 25, 2016

@aciidgh
Copy link
Contributor Author

aciidgh commented Apr 25, 2016

or should it be called TestSuite.xctest ?

@czechboy0
Copy link
Member

I'm having the issue that this fixes here: https://github.com/czechboy0/Redbird
Just clone, run swift build & test on Linux and you should see it. Would really appreciate this fix making it into the next toolchain!

@aciidgh
Copy link
Contributor Author

aciidgh commented Apr 26, 2016

Now I am wondering if its better to pass explicit -module-name flag instead as we try to find this executable via swift-test, will need to append name there too.

@rolivieri
Copy link

Is there an ETA for when this fix will be merged? This is a critical issue as described in https://bugs.swift.org/browse/SR-1243.

@aciidgh
Copy link
Contributor Author

aciidgh commented Apr 28, 2016

@rolivieri I need a confirmation from @mxcl or @ddunbar which way is better here -module-name or renaming the executable, then CI and then we can merge it.

@ddunbar
Copy link
Contributor

ddunbar commented Apr 28, 2016

I think that we should do both:

  • Pass -module-name explicitly.
  • Rename the built bundle. I think the name should be <name>Tests (note plural) for consistency with existing Xcode behavior.

We should also:

  • Have a follow on bug for a verification step in SwiftPM that there is no module name collision across the entire package graph. This is probably even a StarterBug.
  • Figure out why our CI didn't catch this.

@czechboy0
Copy link
Member

czechboy0 commented Apr 28, 2016

@ddunbar I think there are two reasons why CI didn't catch this.

  1. This fails under special circumstances, namely (from the SR bug description)

A helper function is defined inside a class that extends XCTestCase and receives as a parameter a structure defined in the Swift module that is being tested. If the helper function is commented out or the parameter type of the function is changed to, say String, then 'swift test' executes with no errors.

I am fortunate enough to have this issue in Redbird.
And
2. There still isn't a way to run swift test on a fixture in tests, something that #266 would help with. This is also not the first time that swift build succeeded on a fixture, but swift test failed. I think we should have a way to run swift test on a fixture to prevent bugs like these from sneaking in.

@ddunbar
Copy link
Contributor

ddunbar commented Apr 28, 2016

Agreed, can you open an SR on the missing coverage for swift test?

@aciidgh aciidgh force-pushed the rename-xctest-executable branch from dd7e302 to cd397c7 Compare April 28, 2016 16:57
@aciidgh
Copy link
Contributor Author

aciidgh commented Apr 28, 2016

@ddunbar Updated the PR, will file a bug for collision check

@ddunbar
Copy link
Contributor

ddunbar commented Apr 28, 2016

@swift-ci please test

@czechboy0
Copy link
Member

@ddunbar Here's a SR for running swift-test on fixtures: https://bugs.swift.org/browse/SR-1351

@mxcl mxcl merged commit ec7cc38 into swiftlang:master Apr 28, 2016
@aciidgh aciidgh deleted the rename-xctest-executable branch May 10, 2016 06:07
aciidgh pushed a commit to aciidgh/swift-package-manager that referenced this pull request Jan 11, 2019
Refactor Engine/DB Key/KeyID handling
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.

5 participants