-
Notifications
You must be signed in to change notification settings - Fork 180
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
Implement common console launcher #604
Conversation
actual class ConsoleLauncher: AbstractConsoleLauncher() { | ||
override fun parseArgs(args: List<String>): LauncherArgs { | ||
return LauncherArgs( | ||
listOf(ConsoleReporterType(ConsoleReporterType.Format.BASIC)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@charleskorn eventually we need to parse args
here, but for now lets send the default emulating what you had before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me.
} | ||
|
||
data class LauncherArgs( | ||
val reporterTypes: List<ReporterType>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In what scenario would you want to have multiple reporters?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In gradle, sometimes you'd want to generate xml
and html
reports at the same time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, makes sense.
integration-test/build.gradle
Outdated
@@ -28,6 +28,7 @@ kotlin { | |||
dependencies { | |||
implementation project(':spek-dsl') | |||
implementation kotlin('test') | |||
implementation project(':spek-runtime') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed.
} | ||
} | ||
|
||
private fun parseArgs(args: List<String>): LauncherArgs { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@charleskorn I decided to parse the args in a very naive way.
import org.spekframework.spek2.runtime.scope.Path | ||
import org.spekframework.spek2.runtime.scope.ScopeImpl | ||
|
||
data class DiscoveryRequest(val context: DiscoveryContext, val sourceDirs: List<String>, val paths: List<Path>) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense for sourceDirs
to be on the JVM version of DiscoveryContext
rather than on DiscoveryRequest
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, but I'll work on it on a separate PR to minimise the changes here.
private fun parseArgs(args: List<String>): LauncherArgs { | ||
var rawReporterType: String? = null | ||
var rawConsoleReporterFormat: String? = null | ||
var reportExitCode = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this default to true?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
Looks good to me. |
@charleskorn resolves #600
I think the infrastructure/api is mostly there, the only bit is parsing the command line arguments and wiring them up.