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

[WIP] Implement test discovery on linux #2174

Merged
merged 1 commit into from
Jun 25, 2019

Conversation

aciidgh
Copy link
Contributor

@aciidgh aciidgh commented Jun 16, 2019

This is a WIP PR for implementing test discovery on linux using indexing
data. The basic idea is to leverage the IndexStore library to get the
test methods and then generate a LinuxMain.swift file during the build
process.

This PR "works" but there are a number of serious hacks that require
resolving before we can merge it.

Copy link
Contributor

@jakepetroules jakepetroules left a comment

Choose a reason for hiding this comment

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

Please update commit message to "[WIP] Implement test discovery on platforms without an Objective-C runtime (Linux, etc.)"

@@ -186,6 +186,104 @@ extension SPMLLBuild.Diagnostic: DiagnosticDataConvertible {
}
}

class LinuxMainCommand: ExternalCommand {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove all occurrences of "Linux" from the names of these things -- there's nothing inherently Linux-specific about this so it should be named something like "TestMainCommand", "test-main-tool", etc.


public func dlopen(_ path: String?, mode: DLOpenFlags) throws -> DLHandle {
#if os(Windows)
guard let handle = path?.withCString(encodedAs: UTF16.self, LoadLibraryW) else {
Copy link
Contributor

Choose a reason for hiding this comment

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

*W 👍

}

#if !os(Windows)
public func dlerror() -> String? {
Copy link
Contributor

Choose a reason for hiding this comment

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

On Windows you can implement dlerror using GetLastError + FormatMessage. Example here: https://docs.microsoft.com/en-us/windows/desktop/debug/retrieving-the-last-error-code

@aciidgh aciidgh force-pushed the linux-test-discovery branch 8 times, most recently from 4788df3 to 573ff5f Compare June 23, 2019 00:49
…(Linux, etc.)

This is a WIP PR for implementing test discovery on linux using indexing
data. The basic idea is to leverage the IndexStore library to get the
test methods and then generate a LinuxMain.swift file during the build
process.

This PR "works" but there are a number of serious hacks that require
resolving before we can merge it.
@aciidgh aciidgh force-pushed the linux-test-discovery branch from 573ff5f to e36d674 Compare June 23, 2019 00:50
@aciidgh
Copy link
Contributor Author

aciidgh commented Jun 23, 2019

@swift-ci smoke test

}
}

final class TestDiscoveryCommand: CustomLLBuildCommand {
Copy link
Contributor

Choose a reason for hiding this comment

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

Docs would be nice, I have no idea what this command is trying to do :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Going to add some docs in a separate commit.

@aciidgh aciidgh merged commit d65c52c into swiftlang:master Jun 25, 2019
@aciidgh aciidgh deleted the linux-test-discovery branch June 25, 2019 19:14
@weissi
Copy link
Contributor

weissi commented Jun 27, 2019

@aciidb0mb3r would that be something we could have on 5.1 too?

@aciidgh
Copy link
Contributor Author

aciidgh commented Jun 27, 2019

Ya, I think that would be fine since this is currently opt-in.

@weissi
Copy link
Contributor

weissi commented Jun 27, 2019

Ya, I think that would be fine since this is currently opt-in.

Awesome. Are you doing the backport cherry-pick or do you want me to?

@aciidgh
Copy link
Contributor Author

aciidgh commented Jun 27, 2019

I am just bringing the 5.1 branch in sync with master as I plan on making more bug fixes (in unrelated areas) that might make cherry-picking harder: #2196

@weissi
Copy link
Contributor

weissi commented Jul 2, 2019

CC @tomerd btw

@dabrahams
Copy link
Contributor

Is there a way we can turn this on in a Package.swift so that nobody needs to add the flag explicitly?
@compnerd does this feature work on Windows now?

@benlangmuir
Copy link
Contributor

@dabrahams there's a pitch to enable this by default, and the current thinking is to enable it automatically when there is no LinuxMain.swift file present. https://forums.swift.org/t/pitch-enable-test-discovery-by-default/36619/15

@dabrahams
Copy link
Contributor

@benlangmuir So is there no way for me to turn it on in a Package.swift file?

@SDGGiesbrecht
Copy link
Contributor

@dabrahams,

does this feature work on Windows now?

SwiftPM does not exist in the Windows toolchain yet; very little works (mostly because of path handling). That said, there is nothing platform‐specific about this feature, so I imagine it will just work on Windows once the lower‐level issues are resolved. (It certainly worked for Android without anyone needing to put any effort to supporting it.)

So is there no way for me to turn it on in a Package.swift file?

No.

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.

9 participants