Improve shared expression tests#4041
Merged
olemartinorg merged 2 commits intomainfrom Mar 4, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughRefactors the expressions/shared-functions test harness into a data-driven, modular structure with centralized mock factories, a single setupMocks(), and a renderExpression() orchestration; also reorganizes TypeScript type-only imports and adjusts some package/manifest metadata. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
TomasEng
added
the
kind/other
TomasEng
added
ignore-for-release
TomasEng
force-pushed
the
shared-functions-tests-improvements
branch
4 times, most recently
from
64bb054 to
98503bc
Compare
TomasEng
force-pushed
the
shared-functions-tests-improvements
branch
from
98503bc to
a0a602e
Compare
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
TomasEng
added
squad/data
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/features/expressions/shared-functions.test.tsx (1)
294-307: Please align test rendering with the repository test utility guideline.This test now routes through
renderWithoutInstanceAndLayout/renderWithInstanceAndLayoutfor form layout context scenarios. If these do not delegate torenderWithProviders, this diverges from the guideline.As per coding guidelines,
**/*.test.{ts,tsx}: UserenderWithProvidersfromsrc/test/renderWithProviders.tsxwhen testing components that require form layout context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/expressions/shared-functions.test.tsx` around lines 294 - 307, The test is using renderWithoutInstanceAndLayout / renderWithInstanceAndLayout for FormLayout scenarios which may bypass the repo guideline; replace or adjust these calls to use renderWithProviders from src/test/renderWithProviders.tsx (or ensure those helpers delegate to renderWithProviders) so components needing form layout context are wrapped consistently; update the calls in shared-functions.test.tsx (where renderWithoutInstanceAndLayout and renderWithInstanceAndLayout are invoked) to call renderWithProviders with the appropriate initialPage/context (or modify those helpers to internally call renderWithProviders) so the test aligns with the repository test utility guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/expressions/shared-functions.test.tsx`:
- Around line 389-390: The current statefulDataElementId extraction uses a
restrictive regex ([a-f0-9-]+) in the declaration of statefulDataElementId which
fails for non-hex IDs (e.g., 'abc'); update the regex used in the
statefulDataElementId assignment to match any reasonable ID token instead of
only hex (for example use a pattern that captures all characters up to the next
? or & or /), so statefulDataElementId = url.match(/data\/([^?&/]+)\?/)?.[1]
(adjust exact delimiter choices as needed) while leaving statelessDataType
untouched.
- Around line 164-166: The ternary in extractMainTestCase drops valid falsy
expressions like 0, false, and '' because it checks truthiness; update the guard
to only treat null/undefined as missing (e.g., change the condition to check
expression != null or expression !== undefined && expression !== null) so
extractMainTestCase({ expression, expects, expectsFailure }: FunctionTest)
returns { expression, expects, expectsFailure } for falsy-but-valid values;
locate the extractMainTestCase function and adjust its conditional accordingly
to return FunctionTestBase when expression is present even if falsy.
- Around line 174-175: Replace the unsafe type casts in the test setup by
returning fully shaped objects instead of casting: remove casts on
jest.mocked(fetchApplicationMetadata) and jest.mocked(useExternalApis) and
supply a proper ExternalApisResult (e.g., test.externalApis ?? { data: {},
errors: {} }); for places referencing IDataType (the map function at the lines
noted) return objects that include allowedContentTypes, maxCount, and minCount
or annotate the map's return type explicitly so TypeScript infers IDataType
correctly; likewise provide all required IData fields where the IData cast was
used and let inference determine the type for the value at line 361 instead of
casting. Ensure createApplicationMetadata/test helpers produce complete shapes
and update the mocks (fetchApplicationMetadata, useExternalApis) and any map
callbacks to return fully populated objects rather than using "as" casts.
---
Nitpick comments:
In `@src/features/expressions/shared-functions.test.tsx`:
- Around line 294-307: The test is using renderWithoutInstanceAndLayout /
renderWithInstanceAndLayout for FormLayout scenarios which may bypass the repo
guideline; replace or adjust these calls to use renderWithProviders from
src/test/renderWithProviders.tsx (or ensure those helpers delegate to
renderWithProviders) so components needing form layout context are wrapped
consistently; update the calls in shared-functions.test.tsx (where
renderWithoutInstanceAndLayout and renderWithInstanceAndLayout are invoked) to
call renderWithProviders with the appropriate initialPage/context (or modify
those helpers to internally call renderWithProviders) so the test aligns with
the repository test utility guideline.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b6ceeb28-7f13-4d93-962c-5e96df716b64
📒 Files selected for processing (1)
src/features/expressions/shared-functions.test.tsx
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/expressions/shared-functions.test.tsx`:
- Around line 325-327: The regex used to extract the codeListId from url (const
codeListId = url.match(/api\/options\/(\w+)\?/)?.[1];) is too restrictive
because (\w+) excludes hyphens and other valid characters; update that match to
capture a wider set (for example use /api\/options\/([^/?]+)\?/ or
/api\/options\/([-\\w]+)\?/ ) so codeListId correctly matches IDs with hyphens
or non-alphanumeric chars and the subsequent lookup (codeLists[codeListId])
works as expected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c9a00818-2706-4d1a-a1c9-9596f4a8adb2
📒 Files selected for processing (1)
src/features/expressions/shared-functions.test.tsx
github-project-automation
Bot
moved this from 🔎 In review
to ✅ Done
in Team Altinn Studio
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
2 participants
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.
Description
When the
testCasesproperty in the shared expression tests is set, all of the tests are run within the sameitblock. As a consequence, when one of them fails, it is not possible to see which one it is and why it fails. This pull request fixes that, so that each test case is run separately. It also moves some code into separate functions to make it more readable. Most of the changed lines in the diff are indeed just moved from another place.We will need this when implementing the arithmetic operation functions.
Related Issue(s)
Verification/QA
kind/*andbackport*label to this PR for proper release notes groupingSummary by CodeRabbit
Note: Internal test and typing improvements only; no changes to end-user functionality.