Skip to content

test(utils): add edge case tests for format()#5848

Open
Thesmoothengineer wants to merge 2 commits intosugarlabs:masterfrom
Thesmoothengineer:test-format-edge-cases
Open

test(utils): add edge case tests for format()#5848
Thesmoothengineer wants to merge 2 commits intosugarlabs:masterfrom
Thesmoothengineer:test-format-edge-cases

Conversation

@Thesmoothengineer
Copy link
Contributor

@Thesmoothengineer Thesmoothengineer commented Feb 21, 2026

What needs testing

Added additional edge case tests for the format() utility function in utils.js.
Specifically covering:

  • Missing nested properties (e.g. {user.age})

  • Original string return when no placeholders exist

Current coverage

The format() function already had tests for:

  • Simple placeholders
  • Multiple placeholders
  • Nested property access
  • Undefined values

However, missing nested property access and no-placeholder cases were not explicitly covered.

Proposed approach

Added new test cases inside:

js/utils/tests/utils.test.js

The tests ensure:

  • Missing nested properties return an empty string
  • Strings without placeholders remain unchanged

Checklist

  • I have read and followed the project's code of conduct.
  • I have checked that no existing tests cover this area.
  • I have checked that there is no open PR for the tests I am submitting.
  • I have reviewed docs/TESTING.md for testing patterns.

@github-actions
Copy link
Contributor

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

Copy link
Contributor

@kartikktripathi kartikktripathi left a comment

Choose a reason for hiding this comment

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

Tested these changes, all npm test suites pass, meaningful change. Thanks!

@walterbender, This PR is ready to be merged.

@Thesmoothengineer
Copy link
Contributor Author

Thank you for reviewing and testing the changes!

@omsuneri
Copy link
Member

@Thesmoothengineer please run prettier before raiaing PR

@Thesmoothengineer
Copy link
Contributor Author

Sure, I’ll run prettier and update the PR.

@Thesmoothengineer
Copy link
Contributor Author

I’ve run Prettier on the test file and no formatting changes were required. Please let me know if anything else needs to be updated.

@vanshika2720
Copy link
Contributor

@Thesmoothengineer please resolve merge conflicts.

@github-actions
Copy link
Contributor

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:

utils.test.js

@vanshika2720
Copy link
Contributor

@Thesmoothengineer Jest cases are failing.

@Thesmoothengineer
Copy link
Contributor Author

Thesmoothengineer commented Feb 22, 2026

@vanshika2720 The latest CI run is passing.

@github-actions
Copy link
Contributor

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

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.

4 participants