-
Notifications
You must be signed in to change notification settings - Fork 2
@W-20683422 increasing unit test coverage for ODS #32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @charithaT07
Just want to clarify that unit tests of CLI commands should focus on the CLI specifics like flags, expected output (but not strings or tables, test structured output only) and utility functions of the commands. and we should cautious about testing anything that may change in the implementation like the details on how configuration is resolved. I say this because I'm doing exactly that in PR #30, so I'll try to have this merged soon so we can test if it breaks anything here.
The SDK unit tests should provide the coverage for the actual business logic of the services (like ODS) that we want to test, like the rest clients and higher level functions.
The oclif testing libraries might help too: https://oclif.io/docs/testing
These look good just want to confirm we're aligned on the scope of tests. We don't want to be spending too much time refactoring CLI unit tests while the project is in infancy and we're making changes to the SDK. So I want to make sure we focus on the SDK unit tests as a priority until that's stable, then focus on CLI coverage.
(also if we're going to mock by replacing the "client" for CLI tests we should make that a common strategy across all commands and ensure they are developed with that mocking in mind)
Thanks Charles. I updated the ODS CLI unit tests to keep the scope on CLI concerns (flags/command utilities + structured return values) and removed assertions on human-readable strings/tables. We’re consistently mocking by replacing odsClient via shared helpers. And ODS business logic is covered at the SDK level |
This PR adds unit test coverage for the ODS functionality in both the B2C CLI and SDK packages. The implementation validates CLI command logic, configuration settings, output formatting, and SDK client operations using isolated unit tests with mocked dependencies.
Additionally, this PR fixes the ods list command issue where the command would fail when no sandboxes were returned by the API.
Changes Include:
closes #31