SE-0129 test naming conventions #566
Conversation
|
|
||
| /// The module's name is invalid. | ||
| case invalidName(path: String, name: String, problem: ModuleNameProblem) | ||
| enum ModuleNameProblem { |
There was a problem hiding this comment.
nit: invalidTestName / TestModuleNameError would be clearer IMO
There was a problem hiding this comment.
on second look, this seems generic
There was a problem hiding this comment.
Right, this is very much a work in progress. The idea is to overhaul the errors here so that they are specific, and this is definitely something that is going to need to grow over time. I will be adding comments and trying to make these errors (and their fix-its) as specific and clear as possible, but I see the testing module naming as just one aspect of various restrictions on the module name.
|
Yay!! |
| } else { | ||
| modules = try maybeModules.map { path in | ||
| } | ||
| else { |
There was a problem hiding this comment.
This is a nit, but can we use } else { which is most consistent with the rest of the code and Swift?
There was a problem hiding this comment.
Sure. Old habits... I'll search for and change them.
|
Won't this PR fail because it doesn't rename our own tests? |
|
Yes, I am working on those right now. I should have labeled this PR as WIP. |
|
This is also why I haven't yet started the Swift CI test requests for it. |
|
I kind of forgot that people would start looking at it right away (and it was my intent for people to start looking at the core changes, but it was not my intent that this PR would be thought to be "done" yet in terms of fallout). My fault for not tagging it. |
abertelrud
force-pushed
the
SE-0129-Test-Naming-Conventions
branch
from
8288cb0 to
9a88011
Compare
|
@swift-ci please test |
|
Test failures are real but unrelated to this PR; I need to update to the latest trunk. |
|
The conflicts are pervasive, especially due to the recent check-ins related to convention tests. |
|
Fixing, and will then test and merge. |
automatically and silently appending a `TestSuite` suffix, tests are now expected to have a `Tests` suffix in the module name. Right now we make sure there are no tests under `Sources` and no non-tests under `Tests`, but we expect to lift that restriction in the future to allow test support libraries etc. This commit also updates SwiftPM's own tests to follow the new naming conventions.
abertelrud
force-pushed
the
SE-0129-Test-Naming-Conventions
branch
from
9a88011 to
fb5f684
Compare
|
@swift-ci please test |
…g a merge conflict.
|
Local tests on macOS succeed. |
|
Local tests on Linux also succeed. Merging. |
…le requirements. Generate the framework target automatically and manually append any test target. See: <swiftlang/swift-package-manager#566>.
3 participants
Implement SE-0129 to update the test naming conventions. Instead of
automatically and silently appending a
TestSuitesuffix, tests arenow expected to have a
Testssuffix in the module name. Right nowwe make sure there are no tests under
Sourcesand no non-testsunder
Tests, but we expect to lift that restriction in the futureto allow test support libraries etc.