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

Artifact detekt-test foreces dependency constraining #3082

Closed
gfreivasc opened this issue Sep 16, 2020 · 5 comments · Fixed by #3152
Closed

Artifact detekt-test foreces dependency constraining #3082

gfreivasc opened this issue Sep 16, 2020 · 5 comments · Fixed by #3152
Labels
Milestone

Comments

@gfreivasc
Copy link
Contributor

I'm trying to include detekt-test in a project but it fails as it can't find the api dependencies assertj-core and spek-dsl-jvm. It gives me the following message:

Execution failed for task ':detekt-extensions:compileTestKotlin'.
> Could not resolve all files for configuration ':detekt-extensions:testCompileClasspath'.
   > Could not find org.assertj:assertj-core:.
     Required by:
         project :detekt-extensions > io.gitlab.arturbosch.detekt:detekt-test:1.13.1 > io.gitlab.arturbosch.detekt:detekt-test-utils:1.13.1
   > Could not find org.spekframework.spek2:spek-dsl-jvm:.
     Required by:
         project :detekt-extensions > io.gitlab.arturbosch.detekt:detekt-test:1.13.1 > io.gitlab.arturbosch.detekt:detekt-test-utils:1.13.1

Which appears to be because the artifact expects me to use assertJ and speak. My question is, is it really necessary to use both of these libraries to write tests for custom rules? I'm leaning towards simpler jUnit tests, so I had to depend on these libraries to prevent gradle from breaking. Would it be better to not have the api dependencies? Or are these libraries really a requirement?

@mhernand40
Copy link
Contributor

Same for me.

@cortinico
Copy link
Member

Which appears to be because the artifact expects me to use assertJ and speak. My question is, is it really necessary to use both of these libraries to write tests for custom rules?

Some of the utils functions inside detekt-test (and transitively detekt-test-utils) are using those libraries. So if you use those utils functions, you need to import those dependencies. If you wish to use only JUnit, you can obviously do it, but you won't be able to use those util functions.

To name a couple:

setupKotlinEnvironment is using spek:

fun Root.setupKotlinEnvironment() {
val wrapper by memoized(CachingMode.SCOPE, { createEnvironment() }, { it.dispose() })
@Suppress("UNUSED_VARIABLE") // name is used for delegation
val env: KotlinCoreEnvironment by memoized(CachingMode.EACH_GROUP) { wrapper.env }
}

FindingsAssertions is using assertJ:

class FindingAssert(val actual: Finding?) : AbstractAssert<FindingAssert, Finding>(actual, FindingAssert::class.java) {

@gfreivasc
Copy link
Contributor Author

I don't know to what degree there is coupling, but I believe there should at least have some degree of decoupling to hide the dependencies where they're just implementation details and perhaps maintain a separate artifact for the APIs that are more directly coupled. I didn't use the library enough to understand the whole breadth of scenarios it intends to cover and how mush Spek is expected to be present, but I believe there could either be a detekt-test-spek or a lighter detekt-test-core which wouldn't require an explicit dependency on those libraries.

Also, is it possible to have the artifact assume a version in case there's none specified? I believe the dangers of diverging versions are present with or without that assumption anyway.

@cortinico
Copy link
Member

cortinico commented Sep 22, 2020

You're totally right that we should probably expose a detekt-test artifact that is agnostic from the testing framework used. Given that we use internally Spek to run our tests, some of the test utils ended up being coupled with the testing framework.

Agree that we should clean it up a bit and expose as detekt-test-core that is fully agnostic (PRs are more than welcome 🙏 )

EDIT: typo

@arturbosch
Copy link
Member

I've changed the dependencies to compileOnly meaning if the anyone wants to use our assertions or spek integration, he needs to explicitly add the dependency to his project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants