Drop Spek #4670
Drop Spek #4670
Conversation
|
To my understanding (or wishful thinking) we decided to stay with the |
I'm not too bothered personally, it seems the JUnit convention is for Tests (plural) to be the suffix, though that would depend on the number of tests in the class... Also this convention makes sense for inner classes too so perhaps it's not just files and top level classes to rename. I don't think there's anything wrong with Spec though. If the current suffix was Spek that would be different! |
| @@ -11,6 +11,9 @@ import org.spekframework.spek2.dsl.Root | |||
| import org.spekframework.spek2.lifecycle.CachingMode | |||
| import java.nio.file.Path | |||
|
|
|||
| @Deprecated( | |||
| "Use @KotlinCoreEnvironmentTest annotation on test classes instead. See detekt's tests for examples." | |||
There was a problem hiding this comment.
Thoughts on wording? This is only relevant if JUnit 5 is being used. Perhaps this message should be test framework agnostic and/or direct readers to the type resolution testing docs which outlines options for different test frameworks.
There was a problem hiding this comment.
Maybe talks about KotlinCoreEnvironmentTest and createEnvironment so we don't force the use of JUnit 5?
There was a problem hiding this comment.
Agree, this shouldn't be a deprecation imho but more of a KDoc
There was a problem hiding this comment.
Why isn't the deprecation suitable? Are we intending to support those extending detekt and testing with Spek indefinitely?
My preference is a deprecation with removal in the near future (with a copy of the extension function in the release notes that users can then use in their own test code if they're still using Spek).
There was a problem hiding this comment.
Yup sorry, I haven't realized this was Spek-specific. Therefore I'm in for deprecating this
There was a problem hiding this comment.
Take a look at the updated deprecation message - I've directed readers to the type resolution docs and copied the Spek DSL extension code there so it becomes a self-contained example that doesn't rely on detekt-test-utils. This will make it easier to remove the deprecated code in a future release.
3flex
force-pushed
the
drop-spek
branch
2 times, most recently
from
69246d5
to
ab36488
Compare
Codecov Report
@@ Coverage Diff @@
## main #4670 +/- ##
============================================
+ Coverage 84.42% 84.71% +0.28%
+ Complexity 3436 3423 -13
============================================
Files 493 490 -3
Lines 11342 11255 -87
Branches 2085 2069 -16
============================================
- Hits 9576 9535 -41
+ Misses 704 675 -29
+ Partials 1062 1045 -17 Continue to review full report at Codecov.
|
| @@ -11,6 +11,9 @@ import org.spekframework.spek2.dsl.Root | |||
| import org.spekframework.spek2.lifecycle.CachingMode | |||
| import java.nio.file.Path | |||
|
|
|||
| @Deprecated( | |||
| "Use @KotlinCoreEnvironmentTest annotation on test classes instead. See detekt's tests for examples." | |||
There was a problem hiding this comment.
Agree, this shouldn't be a deprecation imho but more of a KDoc
|
I'd like to merge this and prepare 1.20.0 just after it 👍 |
| } | ||
| } | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Do we want to fully drop support for Spek test? Line 129 - 141 still have some code references.
At least we can update them.
There was a problem hiding this comment.
Sorry didn't see your note before pushing an update - I've actually expanded the docs to make deprecation of org.spekframework.spek2.dsl.Root.setupKotlinEnvironment easier for anyone using it in external projects.
The only custom check it included was for Spek, which has been removed from the code base.
The migration has been completed.
These are redundant as Spek is no longer used.
JUnit 5 is recommended, along with the @KotlinCoreEnvironmentTest annotation, to setup a Kotlin environment for testing rules that use type and symbol solving.
Co-authored-by: Goooler <wangzongler@gmail.com>
Co-authored-by: Goooler <wangzongler@gmail.com>
|
LGTM? I'll let someone else click the button after a final review. |
Tying off some loose ends to close #4450.