New unit tests for singleCellCellType & singleCellGeneExpression tws#4300
New unit tests for singleCellCellType & singleCellGeneExpression tws#4300
Conversation
There was a problem hiding this comment.
Pull request overview
Adds initial unit test coverage for the new single-cell termwrapper implementations and aligns termwrapper term.type constants with the shared TermTypes enum-like map.
Changes:
- Added new unit test specs for
tw/singleCellCellTypeandtw/singleCellGeneExpression. - Updated single-cell termwrapper modules to use
TermTypes.*instead of hardcoded strings. - Tightened
singleCellGeneExpressionvalidation to handlenullterms.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| release.txt | Notes the addition of unit tests in the release log. |
| client/tw/test/singleCellGeneExpression.unit.spec.ts | Adds unit tests for getSCGEunit(), validate(), fill(), and constructor behavior. |
| client/tw/test/singleCellCellType.unit.spec.ts | Adds unit tests for validate(), fill(), and constructor defaults. |
| client/tw/singleCellGeneExpression.ts | Switches to TermTypes.SINGLECELL_GENE_EXPRESSION and improves null checking in validate(). |
| client/tw/singleCellCellType.ts | Switches to a TermTypes constant for termType (but currently uses a non-existent key). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| import { TermTypes } from '#shared/terms.js' | ||
|
|
||
| const termType = 'singleCellCellType' | ||
| const termType = TermTypes.SINGLECELL_CELL_TYPE |
There was a problem hiding this comment.
TermTypes does not define SINGLECELL_CELL_TYPE (the existing constant is TermTypes.SINGLECELL_CELLTYPE: 'singleCellCellType'). As written, termType will be undefined, which breaks validation/type checks and can let invalid terms pass silently. Update this to use the correct TermTypes key so termType resolves to 'singleCellCellType'.
| const termType = TermTypes.SINGLECELL_CELL_TYPE | |
| const termType = TermTypes.SINGLECELL_CELLTYPE |
|
|
||
| function getValidRawTerm(overrides: any = {}) { | ||
| return { | ||
| type: TermTypes.SINGLECELL_CELL_TYPE, |
There was a problem hiding this comment.
Tests are using TermTypes.SINGLECELL_CELL_TYPE, but TermTypes defines SINGLECELL_CELLTYPE (no _TYPE). Using the non-existent key makes term.type undefined, which can cause these tests to pass even while the implementation is broken. Replace all occurrences with TermTypes.SINGLECELL_CELLTYPE and consider asserting the expected type string in the thrown error message to prevent this class of regression.
| type: TermTypes.SINGLECELL_CELL_TYPE, | |
| type: TermTypes.SINGLECELL_CELLTYPE, |
…4300) Co-authored-by: xzhou82 <1619109+xzhou82@users.noreply.github.com>
Description
Not urgent. Started new unit test files for
tw/singleCellCellType.tsandtw/singleCellGeneExpression.tsSupports efforts detailed in SC roadmap
Checklist
Check each task that has been performed or verified to be not applicable.