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

Include Kotlin multiplatform test folders to default exclude configuration #2608

Closed
Whathecode opened this issue Apr 14, 2020 · 5 comments · Fixed by #2667
Closed

Include Kotlin multiplatform test folders to default exclude configuration #2608

Whathecode opened this issue Apr 14, 2020 · 5 comments · Fixed by #2667
Milestone

Comments

@Whathecode
Copy link
Contributor

Whathecode commented Apr 14, 2020

The default-detekt-config.yml has several default 'excludes' configurations to exclude test sources.

excludes: '**/test/**,**/androidTest/**,**/*.Test.kt,**/*.Spec.kt,**/*.Spek.kt'

However, this does not cover test folders in Kotlin multiplatform projects, which are named **/<platform-target>Test/**, e.g., **/jsTest/** and **/commonTest/**. I reuse the majority of defaults, but using detekt in a multiplatform project requires me to override all default excludes with my own:

excludes: '**/commonTest/**,**/jvmTest/**,**/jsTest/**,**/test/**'

Therefore, it would be nice to either add the default multiplatform test folders to default-detekt-config.yml, or alternatively, to override the 'default' excludes in one place.

@Whathecode
Copy link
Contributor Author

Maybe **/*Test/**, or would that be too broad?

@BraisGabin
Copy link
Member

BraisGabin commented Apr 14, 2020

'**/test/**' is already too broad... adding **/*Test/** doesn't seem like a huge change. This will help to android and it's flavours. Where the tests of a flavour are placed in **/flavourTest/**.

Any way, I think thatsomthing like #2581 would be really handy for your use case too.

@schalkms
Copy link
Member

schalkms commented May 9, 2020

I think adding **/commonTest/**,**/jvmTest/**,**/jsTest/**,**/iosTest/** makes more sense, since this is really common in Kotlin multi-platform.
If everyone agrees, I'll submit a PR.

@Whathecode
Copy link
Contributor Author

@schalkms I tried making a PR before but couldn't find the sources to modify. :) I presumed that default file was generated?

@schalkms
Copy link
Member

schalkms commented May 9, 2020

One needs to edit 1 file and 1 unit test. The .yml file is auto-generated. I modified the 2 files quickly and submitted the PR.

schalkms added a commit that referenced this issue May 9, 2020
…2667)

* Include Kotlin multiplatform test folders to default exclude config

Closes #2608

* Update sorting of excludes config
@arturbosch arturbosch added this to the 1.8.1 milestone May 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants