Skip to content

Conversation

ViniciusResende
Copy link
Contributor

Context

This PR introduces some small fixes across some of the Cornerstone Extension utility functions and a great amount of unit tests for these utility functions module. This is seen as a improvement in the code quality, enhancing the capability of detecting defects early, also these tests should be kept with care, evolving together with the files.

Changes & Results

  • cd19093, 5566da7, bddda95, 4d07800, 7a420c1, ec04bc5 - These are commits for small fixes that were done during the confection of the tests, they're quite self contained, and the commit itself has a message explaining each one individually.
  • fcaa079, c81e2d5, e5ae00e - Moving JS files to TS.
  • The other commits are all related to addition of unit tests to the Cornerstone Extension utility functions.

Results

The tests are all passing and for most of them 100% of code coverage was reached. For those that do not have this value, it was evaluated that the effort to cover these lines was not compatible with the value got from covering each individual line on those specific files.

image

For some of the functions inside the folder tests were not added now intentionally, because they deserve their own PRs due to it's complexity.

Testing

To test the specific tests that were added one could run: yarn test extensions/cornerstone/src/utils

This will also allow you to check the coverage.

Checklist

PR

  • My Pull Request title is descriptive, accurate and follows the
    semantic-release format and guidelines.

Code

  • My code has been well-documented (function documentation, inline comments,
    etc.)

Public Documentation Updates

  • The documentation page has been updated as necessary for any public API
    additions or removals.

Tested Environment

  • OS: Windows 11 Home 24H2 + WSL Ubuntu 22.04.4 LTS
  • Node version: v20.9.0
  • Browser: Microsoft Edge 139.0.3405.119

This function was using a ES6 spread for cloning the segmentation
parameter, this is a shallow clone and does not apply to nested
structures, therefore this was violating the parameter integrity. By
using clone deep from loadash this problem is fixed.
This is a module that is inside the Cornerstone Extension that is used
as a utilitary function to handle Segmentation Stats update, this commit
adds a test file to it that guarantees 100% of coverage.
Since this function already has a ealy return when the segmentation
isn't a truthy value (line 79), there is no reason to have another if
condition for it, since it will never be satisfied.
This piece of code was removed due to the fact that it was not being
used anymore by the function after its latest changes, the changes
modified the way that the viewports are verified, thus removing the need
for this logic. Since it was not being used anymore, I opted for
removing it so it would not increase the complexity of a new reader
unecessarely.
In the previous implementation when modifying an array (useLists) while
iterating over it with for...of, it can cause elements to be skipped.
When splice removes an element, the remaining elements shift positions,
but the iterator continues from where it was, potentially skipping the
next element. New approach adjust the index after the removal to avoid
this problem.
Copy link

netlify bot commented Oct 2, 2025

Deploy Preview for ohif-dev ready!

Name Link
🔨 Latest commit 8fb8eb9
🔍 Latest deploy log https://app.netlify.com/projects/ohif-dev/deploys/68debf35ef43c6000811c539
😎 Deploy Preview https://deploy-preview-5454--ohif-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant