-
Notifications
You must be signed in to change notification settings - Fork 55
feat: add test for api ref doc analytics #1588
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
Conversation
|
A new generated diff is ready to view.
|
Affected ArtifactsNo artifacts changed size |
|
A new generated diff is ready to view.
|
|
A new generated diff is ready to view.
|
|
A new generated diff is ready to view.
|
ianbotsf
left a comment
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.
Correctness: The test doesn't pass for me:
DokkaAwsTest > testLoadScripts() FAILED
org.opentest4j.AssertionFailedError: Expected script awshome_s_code.js not found
at app//org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:38)
at app//org.junit.jupiter.api.Assertions.fail(Assertions.java:138)
at app//kotlin.test.junit5.JUnit5Asserter.fail(JUnitSupport.kt:56)
at app//kotlin.test.Asserter$DefaultImpls.assertTrue(Assertions.kt:694)
at app//kotlin.test.junit5.JUnit5Asserter.assertTrue(JUnitSupport.kt:30)
at app//kotlin.test.Asserter$DefaultImpls.assertTrue(Assertions.kt:704)
at app//kotlin.test.junit5.JUnit5Asserter.assertTrue(JUnitSupport.kt:30)
at app//kotlin.test.AssertionsKt__AssertionsKt.assertTrue(Assertions.kt:44)
at app//kotlin.test.AssertionsKt.assertTrue(Unknown Source)
at app//aws.sdk.kotlin.dokka.DokkaAwsTest.testLoadScripts(DokkaAwsTest.kt:49)
| @AfterAll | ||
| fun cleanup() { | ||
| if (dokkaOutputDir.exists()) { | ||
| try { | ||
| dokkaOutputDir.deleteRecursively() | ||
| println("Successfully deleted dokka folder") | ||
| } catch (e: Exception) { | ||
| println("Failed to delete dokka folder: ${e.message}") | ||
| } | ||
| } | ||
| } |
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.
Correctness: We shouldn't delete the docs after the test run. Firstly, they're created by Gradle and we shouldn't be messing with Gradle's scope during our tests. More importantly, it makes debugging failures pretty difficult.
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.
That make sense, I will remove cleanup part.
Btw the test not pass is because api ref doc analytic not merged yet.
|
|
A new generated diff is ready to view.
|



Issue #
Description of changes
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.