chore: Rename unitest to test, simplify Framework configuration#313
chore: Rename unitest to test, simplify Framework configuration#313
Conversation
Summary of ChangesHello @xerial, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request is a nice simplification, renaming the unitest sbt project to test and moving the test framework's entry point class to a more logical location. The changes are applied consistently across the build configuration and documentation. I've added a couple of minor suggestions to further improve code style consistency and update a related code comment. Overall, the changes look good.
| // UniTest - Lightweight testing framework with AirSpec syntax | ||
| lazy val unitest = crossProject(JVMPlatform, JSPlatform, NativePlatform) | ||
| lazy val test = crossProject(JVMPlatform, JSPlatform, NativePlatform) |
There was a problem hiding this comment.
For consistency with the project name (uni-test) and the framework name (uni-test), it would be clearer to update this comment to refer to uni-test instead of UniTest.
// uni-test - Lightweight testing framework with AirSpec syntax
lazy val test = crossProject(JVMPlatform, JSPlatform, NativePlatform)
| import sbt.testing.Fingerprint | ||
| import sbt.testing.Framework | ||
| import sbt.testing.Runner | ||
|
|
||
| /** | ||
| * sbt test framework entry point for UniTest | ||
| * sbt test framework entry point for uni-test | ||
| */ | ||
| class UniTestFramework extends Framework: | ||
| override def name(): String = "UniTest" | ||
| class Framework extends sbt.testing.Framework: |
There was a problem hiding this comment.
There's an inconsistent import style. sbt.testing.Framework is used as a fully-qualified name in the class definition, while Fingerprint and Runner from the same sbt.testing package are imported. For better readability and consistency, it's good practice to import all classes used from a package.
I suggest importing sbt.testing.Framework and using its simple name in the class definition.
import sbt.testing.Fingerprint
import sbt.testing.Framework
import sbt.testing.Runner
/**
* sbt test framework entry point for uni-test
*/
class Framework extends Framework:There was a problem hiding this comment.
Thanks for the suggestion, but importing sbt.testing.Framework would cause a naming conflict since our class is also named Framework. The code class Framework extends Framework: would be a recursive definition error. Using the fully qualified name sbt.testing.Framework is intentional to avoid this conflict.
- Rename sbt project variable from `unitest` to `test` for clarity
- Move `UniTestFramework` from spi package to main package as `Framework`
- Simplifies testFrameworks configuration:
`new TestFramework("wvlet.uni.test.Framework")`
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2cb50c0 to
1aeab5b
Compare
Summary
unitesttotest(distinguishes fromuni)UniTestFrameworkfromspipackage to main package asFrameworkChanges
Before:
After:
Test plan
sbt testJVM/testpasses (23 tests)sbt testJS/testpasses (23 tests)sbt testNative/testpasses (23 tests)🤖 Generated with Claude Code