Skip to content

test(utils): consolidate scattered oneHundredToFraction assertions#7443

Open
Harshit-Mishra2212 wants to merge 1 commit into
sugarlabs:masterfrom
Harshit-Mishra2212:test/consolidate-oneHundredToFraction-assertions
Open

test(utils): consolidate scattered oneHundredToFraction assertions#7443
Harshit-Mishra2212 wants to merge 1 commit into
sugarlabs:masterfrom
Harshit-Mishra2212:test/consolidate-oneHundredToFraction-assertions

Conversation

@Harshit-Mishra2212

Copy link
Copy Markdown
Contributor

Description

Following a suggestion by @mahesh-09-12 on #7431 , this PR consolidates the four scattered oneHundredToFraction() describe blocks in utils.test.js into a single, cohesive block for easier maintenance.

Before this change, assertions for the same function were spread across:

  • describe("oneHundredToFraction()")
  • A duplicate describe("oneHundredToFraction()") inside the format() function logic block (exact duplicate of above)
  • describe("oneHundredToFraction() more cases")
  • describe("oneHundredToFraction() exhaustive branch coverage")

All unique assertions are preserved in one consolidated describe("oneHundredToFraction()") block. No assertions were removed except the exact duplicate one.

Related Issue

Follows from reviewer suggestion on #7431 .

PR Category

  • Tests - Updates test coverage

Changes Made

  • js/utils/__tests__/utils.test.js: merge 4 scattered describe blocks into one; remove exact duplicate block; preserve all unique assertions

Testing Performed

  • Ran npm test -- utils.test.js — all tests pass
  • Ran full test suite npm test — no regressions introduced

Checklist

  • I have tested these changes locally and they work as expected.
  • I have updated the documentation to reflect these changes.
  • I have followed the project's coding style guidelines.
  • I have run npm run lint and npx prettier --check . with no errors.
  • I have addressed the code review feedback from the previous submission.
  • I have enabled "Allow edits from maintainers"

Additional Notes for Reviewers

No production code was changed; only utils.test.js is touched. All 18 unique test values are preserved in the consolidated block.

@github-actions github-actions Bot added tests Adds or updates test coverage size/M Medium: 50-249 lines changed area/javascript Changes to JS source files area/tests Changes to test files labels May 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🧪 Jest Test Results

✅ All Jest tests passed! This PR is ready to merge.

Coverage: Statements: 48.09% | Branches: 39.61% | Functions: 52.88% | Lines: 48.49%
Master Coverage: Statements: 48.09% | Branches: 39.61% | Functions: 52.88% | Lines: 48.49%

@mahesh-09-12 mahesh-09-12 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for following up on the maintainability concern - my earlier suggestion was mainly about the duplicated assertions, but consolidating the scattered blocks also looks cleaner overall.

@vyagh vyagh requested review from Ashutoshx7 and omsuneri June 11, 2026 13:01
@Ashutoshx7

Copy link
Copy Markdown
Collaborator

-Could you update the title/description to reflect that
It'd avoid confusion for anyone reading the history later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/javascript Changes to JS source files area/tests Changes to test files size/M Medium: 50-249 lines changed tests Adds or updates test coverage

Projects

Development

Successfully merging this pull request may close these issues.

3 participants