-
Notifications
You must be signed in to change notification settings - Fork 39
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
Refactor DiktatMavenPluginIntegrationTest
to ease troubleshooting in case of test failures
#1304
Refactor DiktatMavenPluginIntegrationTest
to ease troubleshooting in case of test failures
#1304
Conversation
…n case of test failures ### What's done: * `TestInfo` is now passed to test methods so that `jacoco-it-*.exec` files are unique. * `jacoco-it-*.exec` files are now deleted before each test, so repeated test invocations no longer cause failures. * Added `String.assertContains()` extensions so that assertion failures have meaningful messages. * Proofread and expanded the KDoc comment.
* | ||
* @param other the expected substring. | ||
*/ | ||
private inline fun String.assertContains(other: CharSequence, |
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 thought that assertContains
is there in kotlin.test
. Why do we need these?
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.
Indeed. But we don't have it added to our pom.xml
yet. Should I add kotlin-test
instead?
...aven-plugin/src/test/kotlin/org/cqfn/diktat/plugin/maven/DiktatMavenPluginIntegrationTest.kt
Outdated
Show resolved
Hide resolved
* * maven execution results are analyzed here; `.mvn/jvm.config` is used to attach jacoco java agent to every maven process and generate individual execution reports. | ||
* | ||
* When within an IDE (e.g.: _IDEA_), don't run this test directly: it's | ||
* expected to be executed from a forked JVM. Instead, run a `verify` _lifecycle |
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.
Did you have a chance to try to run this forked JVM with remote debugging support? Such instructions would be useful here too
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.
Yeah, some notes about the remote debugger in the case of diktat will be great here.
But actually I love ultimate debugging tool called print
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's on my TODO list, will update the KDoc.
FYI: you can run |
…en/DiktatMavenPluginIntegrationTest.kt Co-authored-by: Peter Trifanov <peter.trifanov@mail.ru>
Codecov Report
@@ Coverage Diff @@
## master #1304 +/- ##
=========================================
Coverage 82.01% 82.01%
Complexity 2530 2530
=========================================
Files 105 105
Lines 7201 7201
Branches 2058 2058
=========================================
Hits 5906 5906
Misses 346 346
Partials 949 949
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
### What's done: * Indentation fixed with diktat:fix@diktat
### What's done: * Switched to AssertJ and dropped the extensions
What's done:
TestInfo
is now passed to test methods so thatjacoco-it-*.exec
filesare unique.
jacoco-it-*.exec
files are now deleted before each test, so repeated testinvocations no longer cause failures.
String.assertContains()
extensions so that assertion failures havemeaningful messages.