-
Notifications
You must be signed in to change notification settings - Fork 20
Reuse kt icons across tests #799
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request refactors test resources and test cases in the image vector parser module. The changes include renaming test resources and test cases from "EmptyImageVector" to "WithoutPath", removing obsolete test resource files for EmptyImageVector and the original AllPathParams definitions, and introducing a new ExpectedWithoutPathImageVector test expectation file. Additionally, the AllPathParams test resource files in both lazy and backing implementations are updated with significantly modified path drawing command sequences. The expected test file for AllPathParams is updated to include the Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (11)
💤 Files with no reviewable changes (4)
🧰 Additional context used🧠 Learnings (3)📚 Learning: 2025-10-21T20:55:27.073ZApplied to files:
📚 Learning: 2025-12-07T20:07:49.753ZApplied to files:
📚 Learning: 2025-09-27T22:19:48.762ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdk/intellij/psi/imagevector/src/test/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/imagevector/KtFileToImageVectorParserTest.kt (1)
64-83: Migrate IconWithGroup and SinglePath test resources to shared location.Most tests in this file use the new shared resource path pattern (
imagevector/kt/lazy/andimagevector/kt/backing/), butIconWithGroupandSinglePathtests still reference the old local paths (lazy/andbacking/). The test files exist only in the old locations; they should either be migrated tosdk/test/sharedTestResources/imagevector/kt/to maintain consistency with other tests, or there should be a clear reason to keep them isolated.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
sdk/intellij/psi/imagevector/src/test/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/imagevector/KtFileToImageVectorParserTest.ktsdk/intellij/psi/imagevector/src/test/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/imagevector/expected/AllPathParams.ktsdk/intellij/psi/imagevector/src/test/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/imagevector/expected/ExpectedWithoutPathImageVector.ktsdk/intellij/psi/imagevector/src/test/resources/backing/AllPathParams.ktsdk/intellij/psi/imagevector/src/test/resources/backing/EmptyImageVector.ktsdk/intellij/psi/imagevector/src/test/resources/lazy/AllPathParams.ktsdk/intellij/psi/imagevector/src/test/resources/lazy/EmptyImageVector.ktsdk/test/sharedTestResources/imagevector/kt/backing/AllPathParams.ktsdk/test/sharedTestResources/imagevector/kt/backing/EmptyPaths.ktsdk/test/sharedTestResources/imagevector/kt/lazy/AllPathParams.ktsdk/test/sharedTestResources/imagevector/kt/lazy/EmptyPaths.kt
💤 Files with no reviewable changes (4)
- sdk/intellij/psi/imagevector/src/test/resources/lazy/EmptyImageVector.kt
- sdk/intellij/psi/imagevector/src/test/resources/lazy/AllPathParams.kt
- sdk/intellij/psi/imagevector/src/test/resources/backing/EmptyImageVector.kt
- sdk/intellij/psi/imagevector/src/test/resources/backing/AllPathParams.kt
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-21T20:55:27.073Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 651
File: tools/idea-plugin/build.gradle.kts:147-175
Timestamp: 2025-10-21T20:55:27.073Z
Learning: In Gradle Kotlin DSL (.gradle.kts) scripts, the types `org.gradle.api.artifacts.ArtifactCollection` and `org.gradle.api.artifacts.component.ModuleComponentIdentifier` are implicitly available and do not require explicit import statements.
Applied to files:
sdk/intellij/psi/imagevector/src/test/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/imagevector/KtFileToImageVectorParserTest.kt
📚 Learning: 2025-12-07T20:07:49.753Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 750
File: tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/task/GenerateImageVectorsTask.kt:71-85
Timestamp: 2025-12-07T20:07:49.753Z
Learning: In the Valkyrie Gradle plugin (Kotlin), the `useFlatPackage` flag in `IconPackExtension` is only applicable when nested packs are configured. For single icon packs (without nested packs), the flag is intentionally not propagated to `ImageVectorGeneratorConfig` as there is no package hierarchy to flatten.
Applied to files:
sdk/intellij/psi/imagevector/src/test/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/imagevector/KtFileToImageVectorParserTest.kt
📚 Learning: 2025-09-27T22:19:48.762Z
Learnt from: t-regbs
Repo: ComposeGears/Valkyrie PR: 587
File: sdk/generator/xml/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/generator/xml/XmlSerializer.kt:13-16
Timestamp: 2025-09-27T22:19:48.762Z
Learning: The VectorDrawable.Child sealed interface in the Valkyrie project only has Group and Path as implementations. ClipPath is not implemented in this project, despite being supported in Android's official VectorDrawable specification.
Applied to files:
sdk/intellij/psi/imagevector/src/test/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/imagevector/expected/ExpectedWithoutPathImageVector.ktsdk/test/sharedTestResources/imagevector/kt/backing/AllPathParams.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (5)
sdk/intellij/psi/imagevector/src/test/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/imagevector/expected/AllPathParams.kt (1)
18-18: LGTM!The addition of
autoMirror = trueextends test coverage to include this builder parameter. The path commands in the expected file are consistent with the corresponding lazy and backing test resources.sdk/test/sharedTestResources/imagevector/kt/lazy/AllPathParams.kt (1)
34-52: LGTM - comprehensive path operation coverage.The expanded path commands provide exhaustive test coverage for all path operation types including relative/absolute variants. The mix of named parameters (line 49:
isMoreThanHalf = false, isPositiveArc = false) and positional booleans (line 50:true, false) forarcToeffectively tests both syntax styles that the parser must handle.sdk/intellij/psi/imagevector/src/test/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/imagevector/expected/ExpectedWithoutPathImageVector.kt (1)
6-12: LGTM!The rename from
ExpectedEmptyImageVectortoExpectedWithoutPathImageVectorimproves clarity by explicitly describing what the test vector lacks, rather than the ambiguous "empty".sdk/intellij/psi/imagevector/src/test/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/imagevector/KtFileToImageVectorParserTest.kt (1)
31-38: LGTM!The test rename and path updates correctly align with the new shared test resource structure.
sdk/test/sharedTestResources/imagevector/kt/backing/AllPathParams.kt (1)
38-56: LGTM!The backing implementation correctly mirrors the lazy version's path commands, ensuring consistent test behavior across both initialization patterns.
No description provided.