test: add unit tests across pure utilities, forms, and ORCID helpers#880
Open
thostetler wants to merge 10 commits into
Open
test: add unit tests across pure utilities, forms, and ORCID helpers#880thostetler wants to merge 10 commits into
thostetler wants to merge 10 commits into
Conversation
Move percent-encoding from the DOI pre-encoding site to the map level so that all field values—including rft.atitle, rft.jtitle, rfr_id, etc.—are encoded exactly once, preventing raw special characters from breaking the query string.
Cover graphUtils transforms, visualization utils, ORCID model type guards, email notification keyword validator, and Nivo dark theme shape — all pure functions with no mocking required.
Cover CitationForm author/ORCID validation and submit payloads, KeywordsForm debounced validation and mutation flow, ORCID query key helpers, and useBubblePlot scale construction including log-zero normalization and empty/single-item edge cases.
Cover ArxivForm parent/child category toggle behavior and submit payload collapsing, plus pure helpers from useRemoveWorks: getFulfilled, getRejected, and findPutcodeInProfile including numeric putcode coercion.
Cover convertDocType all mappings, findWorkInProfile nil/array/string lookups, mergeOrcidMissingRecords deduplication, and QueryForm submit flow including vault search trigger, success/error paths, and frequency selection.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #880 +/- ##
========================================
+ Coverage 61.2% 64.1% +2.9%
========================================
Files 348 349 +1
Lines 41373 41385 +12
Branches 1826 2016 +190
========================================
+ Hits 25297 26500 +1203
+ Misses 16033 14842 -1191
Partials 43 43
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR primarily increases unit test coverage across low-coverage, deterministic areas (pure utilities, visualization transforms, ORCID helpers, and EmailNotification forms). It also introduces/adjusts URL-encoding behavior for DOI/OpenURL generation to ensure generated links are safe for use in URL paths and query strings.
Changes:
- Added extensive Vitest coverage for visualization utils, ORCID helpers/models, EmailNotification forms, and theme/hooks.
- Introduced
encodeDOIPathand applied DOI-safe path encoding to DOI links in JSON-LD metatags and ADS gateway DOI URLs. - Updated OpenURL query construction to encode query parameter values (and adjusted tests accordingly).
Risk summary
Medium risk: one production helper (createUrlByType) now uses DOI-specific encoding for all identifier types, which can produce malformed gateway URLs for non-DOI identifiers in edge cases.
Findings (priority order)
medium
- Non-DOI identifiers may be encoded incorrectly in gateway links
- Impact: Gateway URLs generated for non-DOI identifier types (e.g., arXiv) may end up with unintended path segmentation (notably when identifiers contain
/, such as legacy arXiv IDs). This can break outbound links in the Abstract details UI. - Location:
src/components/AbstractSources/linkGenerator.ts(createUrlByType, around lines 141–144) - Minimal fix: Encode DOI identifiers with
encodeDOIPath, but encode all other identifiers withencodeURIComponent, and alsoencodeURIComponentthe bibcode for consistency. - Confidence: high
- QueryForm Select mock can pass
undefinedtoonChange(type-check/runtime hazard)
- Impact: The test mock can fail TypeScript checking and (in edge cases) can call
onChange(undefined), diverging from the component’s expected onChange shape. - Location:
src/components/EmailNotifications/Forms/QueryForm.test.tsx(mocked SelectonChange, around line 47) - Minimal fix: Use a non-null assertion (
!) or handle the undefined case explicitly. - Confidence: high
low
3) Misleading test description for percent encoding behavior
- Impact: Test name/comment suggests “no double-encoding,” but the expectation demonstrates percent (
%) is encoded to%25(so%23becomes%2523). This can confuse future maintainers. - Location:
src/utils/common/__tests__/encodeDOI.test.ts(around lines 27–30) - Minimal fix: Rename the test to accurately describe encoding literal
%. - Confidence: high
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/common/encodeDOI.ts | Adds encodeDOIPath utility for DOI-safe URL path encoding (preserving /). |
| src/utils/common/tests/encodeDOI.test.ts | Unit tests for encodeDOIPath covering reserved characters and % handling. |
| src/lib/useNivoDarkTheme.test.ts | Tests expected Nivo dark theme structure and specific colors. |
| src/lib/orcid/useRemoveWorks.test.ts | Tests pure helpers used by useRemoveWorks (fulfilled/rejected extraction, putcode lookup). |
| src/lib/orcid/helpers.test.ts | Tests ORCID helper functions (doc type conversion, profile lookup, merge behavior). |
| src/components/Visualizations/utils/utils.test.ts | Tests visualization utility functions (ticks, grouping, query wrapping). |
| src/components/Visualizations/utils/graphUtils.test.ts | Large unit test suite for graph data transformers and plotting utilities. |
| src/components/Visualizations/Graphs/useBubblePlot.test.tsx | Hook tests for scale construction, theming, and edge cases (empty data/log scales). |
| src/components/Metatags/json-ld-abstract/identifiers.ts | Uses encodeDOIPath when generating DOI sameAs URLs. |
| src/components/Metatags/json-ld-abstract/tests/identifier.test.ts | Adds test ensuring DOI sameAs URL encodes reserved characters. |
| src/components/EmailNotifications/Forms/Utils.test.tsx | Tests keyword validation helper behavior. |
| src/components/EmailNotifications/Forms/QueryForm.test.tsx | Tests QueryForm submission/state machine and vault search integration (mocked). |
| src/components/EmailNotifications/Forms/KeywordsForm.test.tsx | Tests keyword form debounce validation and submit behavior. |
| src/components/EmailNotifications/Forms/CitationForm.test.tsx | Tests citation form validation and mutation payload construction. |
| src/components/EmailNotifications/Forms/ArxivForm.test.tsx | Tests arXiv category selection UX and submission payload collapsing. |
| src/components/AbstractSources/openUrlGenerator.ts | Encodes OpenURL query parameter values with encodeURIComponent (behavior change). |
| src/components/AbstractSources/linkGenerator.ts | Uses DOI-safe encoding for identifiers when building gateway URLs (behavior change). |
| src/components/AbstractSources/tests/openUrlGenerator.test.ts | Adds OpenURL encoding regression test for # in DOI. |
| src/components/AbstractSources/tests/linkGenerator.test.ts | Updates expected OpenURL strings to match encoded query values. |
| src/components/AbstractSources/tests/createUrlByType.test.ts | New tests verifying DOI encoding behavior in gateway DOI links. |
| src/api/orcid/orcid.test.ts | Tests ORCID React Query key construction (ensuring user/token excluded). |
| src/api/orcid/models.test.ts | Tests ORCID validators (IDs, user objects, profile entry shape). |
Comment on lines
141
to
144
| export const createUrlByType = function (bibcode: string, type: string, identifier: string): string { | ||
| if (typeof bibcode === 'string' && typeof type === 'string' && typeof identifier === 'string') { | ||
| return `${GATEWAY_BASE_URL + bibcode}/${type}:${identifier}`; | ||
| return `${GATEWAY_BASE_URL + bibcode}/${type}:${encodeDOIPath(identifier)}`; | ||
| } |
Comment on lines
+27
to
+30
| it('does not double-encode already-encoded sequences', () => { | ||
| // % itself gets encoded to %25, preventing double-encoding | ||
| expect(encodeDOIPath('10.1000/foo%23bar')).toBe('10.1000/foo%2523bar'); | ||
| }); |
Comment on lines
+44
to
+48
| aria-label={typeof label === 'string' ? label : 'select'} | ||
| data-testid={id} | ||
| value={String(value?.id ?? '')} | ||
| onChange={(event) => onChange(options.find((option) => String(option.id) === event.target.value))} | ||
| > |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overall test coverage was at 62.57%. This adds 156 new tests across 11 files targeting the lowest-coverage areas with deterministic, pure-function behavior.