[6.0] Prefix module name to test #1546
Conversation
It is possible to have two identically named suites in two different test targets. These were being erroniously rolled up in to the same parent TestItem. Disambiguate these TestItems by prepending the module name. This has the added benefit of making the TestItem IDs a fully qualified name that can be passed to `swift test`. The module name is pulled from the compiler arguments for the target. If no module name can be found we fall back to the `targetID` for the `ConfiguredTarget`.
plemarquand
force-pushed
the
prefix-module-name-to-test-id
branch
from
c49d6a9 to
1abfb65
Compare
| @@ -154,6 +155,37 @@ extension BuildSystemManager { | |||
| .first | |||
| } | |||
|
|
|||
| /// Returns the target's module name as parsed from the `ConfiguredTarget`'s compiler arguments. | |||
| public func moduleName(for document: DocumentURI, in target: ConfiguredTarget) async -> String? { | |||
There was a problem hiding this comment.
This feels like something that should be provided by the build system, rather than found from the settings after the fact. We could have a default that does this though.
It's also the last argument that wins, so both the .first* below should be .last* instead.
There was a problem hiding this comment.
Its my understanding that the build settings already give us a standardized way of getting the compilerArguments across all the difference build systems, so pushing the implementation down into each individual BuildSystem would be largely duplicating the implementation of BuildSystem.buildSettings(for:in:language:). Let me know if this isn't what you meant.
Great catch on argument precedence, I'll put up a patch that fixes it on main and then roll it in to this cherry-pick.
There was a problem hiding this comment.
I've created #1549 to fix argument precedence
There was a problem hiding this comment.
Its my understanding that the build settings already give us a standardized way of getting the compilerArguments across all the difference build systems
This is true, but doesn't necessarily mean we should go searching through build settings for the information we want (I would consider that a fallback) - the build system may already have the information needed. And like I said, we could always have a default implementation for that method that does as you are now.
|
Feel free to just cherry-pick the update in this one too |
|
@swift-ci Please test |
workspace/testsandtextDocument/testsrequests