-
Notifications
You must be signed in to change notification settings - Fork 0
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
Migrate (set_once)
tests to Kotlin
#144
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #144 +/- ##
=========================================
Coverage 34.21% 34.21%
Complexity 360 360
=========================================
Files 134 134
Lines 2709 2709
Branches 220 220
=========================================
Hits 927 927
Misses 1719 1719
Partials 63 63 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see my comments.
@CheckReturnValue | ||
@ParametersAreNonnullByDefault | ||
package io.spine.test.tools.validate.setonce; | ||
internal object SetOnceAssertions { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need an object here? Why cannot we have just top level functions?
/** | ||
* Asserts that the given [runnable] throws [ValidationException]. | ||
*/ | ||
fun assertValidationFails(runnable: () -> Unit) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This applies not only to setonce
. So if we want these functions, they should probably go as test fixtures at the very start level of project dependencies.
/** | ||
* Asserts that the given [runnable] doesn't throw anything. | ||
*/ | ||
fun assertValidationPasses(runnable: () -> Unit) = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Names of these functions are a bit long to be utilities. We also seem to have already things like assertValid()
and assertInvalid()
. Please have a look around.
* Asserts that the given [runnable] doesn't throw anything. | ||
*/ | ||
fun assertValidationPasses(runnable: () -> Unit) = | ||
assertDoesNotThrow(runnable) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is also useful to return a result of the executing the runnable
. It helps when you need to verify the result of building a message. We also have tests that run .validate()
in combination with just building a message.
Again, please have a look around the code of the project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean the code like this:
val msg = assertDoesNotThrow {
protoSet {
element.add(toAny("42"))
element.add(toAny(42))
}
}
msg.validate().shouldBeEmpty()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we introduce something like assertValid()
it may be implemented as the code above. But, again, we need to return the produced message so that the tests that use the utility can do something useful with it without the "gymnastics" with var
declarations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've introduced them within an object as (set_once)
-specific intentionally.
- Assertions within
IsValid
are not applicable as they rely on validation being done inbuilder()
method. The test subject here throws right from a builder setter. - Within
(set_once)
tests, we don't need the results, so I don't return them, allowing the test cases be one-lines.
I didn't look at them as general-purpose. I can move them back to Spec*
files (two files need them), and make private
. Or we can discuss vocally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, assuming agreements we've made in the video call.
This PR migrates recently added
(set_once)
tests to Kotlin.