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

Fix kotlin tests #24

Merged
merged 6 commits into from Jul 10, 2018
Merged

Fix kotlin tests #24

merged 6 commits into from Jul 10, 2018

Conversation

tonilopezmr
Copy link
Contributor

@tonilopezmr tonilopezmr commented Jun 21, 2018

Motivation

While using the plugin, we have found that when you run sbt test the plugin isn't able to find the test classes. This doesn't let us execute our test written in kotlin.

Implementation

I have added a KotlintTest class to find kotlin test classes, KotlinTest class should be located in the sbt package to access to some sbt protected objects and methods like sbt.classfile.Analyze

Usage

To be able to run the test using this plugin you only need to add the definedTests in Test := KotlinTest.kotlinTests.value line in build.sbt.

lazy val `project-name` = (project in file("."))
  .settings(
    kotlinLib("stdlib"),
    definedTests in Test := KotlinTest.kotlinTests.value
  )

@pfn
Copy link
Owner

pfn commented Jul 3, 2018

It would be great if this PR included a scripted test that illustrates this working.

README.md Outdated
@@ -40,6 +40,7 @@ Current version 1.0.8
`kotlinVersion`
plugin
* `kotlincOptions`: options to pass to the kotlin compiler
* `definedTests in Test := KotlinTest.kotlinTests.value`: specifies kotlin test classes
Copy link
Owner

Choose a reason for hiding this comment

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

this seems unnecessary to document unless there's a specific user-need that they would want to override this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this line doesn't have sense because I have added in the kotlinSettings. I will remove it. 💯

@@ -68,6 +68,7 @@ object KotlinPlugin extends AutoPlugin {
classDirectory.value, streams.value)
}.dependsOn (compileInputs in (Compile,compile)).value,
compile := (compile dependsOn kotlinCompile).value,
kotlinSource := sourceDirectory.value / "kotlin"
kotlinSource := sourceDirectory.value / "kotlin",
definedTests in Test := KotlinTest.kotlinTests.value
Copy link
Owner

Choose a reason for hiding this comment

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

will this clobber existing definitions of definedTests? if so, perhaps the correct approach would be to use ++= rather than :=


object KotlinTest {
val kotlinTests = Def.task {
val old = (definedTests in Test).value
Copy link
Owner

Choose a reason for hiding this comment

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

I see why := was used in the settings. I'd rather this switch to ignoring old and using ++= instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you are right, It was my first version ^^

@tonilopezmr
Copy link
Contributor Author

I did the changes you suggested and I added a two tests basic-tests and mixed-tests with a test example using JUnit 👍

val loader = ClasspathUtilities.toLoader((fullClasspath in Test).value map {
_.data
})
val log = sLog.value
Copy link
Owner

Choose a reason for hiding this comment

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

oops, didn't notice this before, instead use streams.value.log

@@ -0,0 +1,3 @@
> compile
> test
> "test-only *SimpleTest"
Copy link
Owner

Choose a reason for hiding this comment

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

It would be great if you could also verify that target/test-reports/SimpleTest.xml and target/test-reports/demo.JavaCalculator.xml gets created with expected contents

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@@ -0,0 +1,3 @@
> compile
> test
> "test-only *MixedTest"
Copy link
Owner

Choose a reason for hiding this comment

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

ditto previous verification comment

@tonilopezmr
Copy link
Contributor Author

I have checked if there is something failure test 👍

@pfn pfn merged commit 2d50715 into pfn:master Jul 10, 2018
tonilopezmr added a commit to Karumi/play-framework-kotlin that referenced this pull request Aug 22, 2018
* Show an example code in Play Framework using sbt in kotlin.
* Uses the new version of [kotlin-plugin for sbt](https://github.com/pfn/kotlin-plugin) we implemented here pfn/kotlin-plugin#24 to run kotlin tests.
* Show running test in kotlin with sbt
* Database tests using H2
* End to End tests using H2
* Uses [karumi/ktlint-sbt](https://github.com/Karumi/ktlint-sbt) plugin
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.

None yet

2 participants