-
Notifications
You must be signed in to change notification settings - Fork 354
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
improve kotlin support #260
Comments
As I already pointed out in my comment to Nicolas`s blog post you've mentioned it would be even better if the Kotlin guys could add line information to their when-constructs. Did you ask them or file a ticket at JetBrains? A line number will be required to identify mutations and track line coverage - maybe we could guess the line number or just assume line zero but that might produce bad results with multiple when-constructs in a single class. But I didn't take a serious look into the code required to change. Regards |
I asked in the kotlinlang slack, and they said its possible, so i created a youtrack ticket: |
I looked at the pitest source and it seems the error message is a bit misleading. Pitest runs fine without line number info, but wants a source file info for every class. pitest/pitest/src/main/java/org/pitest/mutationtest/verify/DefaultBuildVerifier.java Line 61 in f6ebe06
The WhenMapping class that kotlin generates is marked as synthetic, so maybe pitest can just ignore synthetic classes without associated source file. |
@christophsturm Thanks for the PR. Not performing the check for synthetic classes sounds like the right solution as long as no directly source derived bytecode is ending up in those classes. I took a quick look at a XXX$WhenMappings class and this certainly looks to be the case. |
thanks for the quick fix! are you planning a new release soon? |
It's been a while since the last release so no objection to doing one soon. There won't be very much in it at the moment however. If you have time to run the snapshot release over a few kotlin codebases we could see if there are any other easy to fix kotlin issues that could be addressed first? |
ok will do! |
Just took a quick look at this myself. I don't know much about Kotlin, but was aware that it had something like Scala's case classes - which turned out to be data classes. PIT generated a lot of junk mutations for these due to all the autogenerated methods. I've out together a quick fix that will filter these out. It isn't pretty, it just looks for any mutation on line 0 of a class compiled from a file ending in .kt, but it should catch a lot of the junk. It looks like it might be possible to do something a little cleaner using the data in the kotlin.MetaData annotation that seems to be added to each class. I'm not entirely clear what's stored in there yet though. |
that fix should also remove this mutation:
|
it seems if i run mvn clean org.pitest:pitest-maven:mutationCoverage pitest does not find any tests. mvn test org.pitest:pitest-maven:mutationCoverage works. I think the reason is that the pitest task depends on the java compile task, but in this case it should depend on the kotlin compile task. Or I'm doing it wrong, I usually use pitest via the gradle plugin :) btw I'm using the ktor project if you want to reproduce |
If you run a goal directly in maven you need to manually ensure any goals you depend on have already run. If you bind the goal you want to a phase then maven takes care of making sure everything happens. So you'll need to run test first unless you add pitest into the pom. |
this code: |
We are also thinking about using PIT with Kotlin. I have asked the JetBrains team if it'd be possible to annotate auto-generated messages in the compiler. https://discuss.kotlinlang.org/t/annotating-auto-generated-methods/1693 |
@mikehearn That would certainly make life very easy at this end, though I'm not sure the kotlin team will want to increase the binary size. I've not spent enough time looking at what kotlin produces to understand if all the the scaffolding code will be cleanly off in its own methods or if some of it will be intermingled with user code. |
@hcoles Tried again with this new version and it works much better. Many thanks! |
I added a youtrack issue to track this https://youtrack.jetbrains.com/issue/KT-13102 |
Kotlin classes that call Java methods that never return I tried the naive approach of adding |
@christophsturm PR that's been left unmerged for some time now (sorry chris) would fix this, however the code base has moved on somewhat since he raised it. The new MutationInterceptor plugin is now the preferred way to deal with this kind of thing. It should be straightforward to port Chris' work across though. |
i will revisit my pr this week. been living without mutation testing too long anyway |
Hey guys! Any news on this? |
I did in fact do a bit of work on a kotlin plugin a few weeks ago. I'm struggling now to remember what I did with it and what state it was in. |
@hcoles well, for me even @christophsturm -style hack will work :) |
I also looked at my branch yesterday. But I agree that it should be a
plug-in. Can you post what you have to make sure we don’t do duplicate work?
Paul <[email protected]> schrieb am Mo. 8. Jän. 2018 um 21:15:
… @hcoles <https://github.com/hcoles> well, for me even @christophsturm
<https://github.com/christophsturm> -style hack will work :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#260 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAAHzzGYWUQ1XMEHM3UQ7zFjXnSXcKtrks5tIndxgaJpZM4IGQG9>
.
|
I'll post it when I've found it. |
Ok, it's uploaded here https://github.com/pitest/pitest-kotlin I'm afraid I can't remember what state I left it in or how scrappy a job I was doing, but help on getting into a usable state would be very much appreciated. I don't think I did anything to deal with the calls to internal.intrinsics - but that should be very easy to add. Pitest 1.3.0 also included an important change to improve kotlin support. The new return value mutators now pay attention to NotNull annotations (which kotlin adds), and will not mutate these methods to return null. These are not currently activated by default so kotlin users should use the NEW_DEFAULTS mutator set (or their own set that excludes the RETURN_VALS mutator). The plan is to make this the default behaviour if no-one reports major issues with the new set. |
Thank you very much!
As of now I can't access pitest-kotlin, but it was enough to enable
NEW_DEFAULTS and add
```xml
<avoidCallsTo>
<avoidCallsTo>kotlin.jvm.internal</avoidCallsTo>
</avoidCallsTo>
```
to maven config to disable these obsolete checks
|
For those who use a gradle, you can try this: // thanks @asm0dey pitest {
targetClasses = ["our.base.package.*"]
pitestVersion = '1.4.0'
avoidCallsTo = ['kotlin.jvm.internal']
} |
PIT also seems to have trouble with Kotlin's coroutines. This simple code sample: private fun test() {
GlobalScope.launch {
}
} Will create bunch of |
@matejdro maybe this should be a separate issue. kotlin support in pitest is pretty good already. |
Okay, I thought this is umbrella Kotlin ticket. I have created separated issue |
Kotlin support is now provided by https://docs.arcmutate.com/docs/kotlin |
according to this article pitest works pretty good with kotlin https://blog.frankel.ch/experimental-kotlin-mutation-testing
if there was a flag to turn this error:
into a warning, it would work even better.
The text was updated successfully, but these errors were encountered: