Skip to content

feat: add CoreExternalDevice API coverage#371

Open
jmcte wants to merge 2 commits intoN4S4:masterfrom
OMT-Global:pr/core-external-device
Open

feat: add CoreExternalDevice API coverage#371
jmcte wants to merge 2 commits intoN4S4:masterfrom
OMT-Global:pr/core-external-device

Conversation

@jmcte
Copy link
Copy Markdown
Contributor

@jmcte jmcte commented Apr 17, 2026

Summary

  • add CoreExternalDevice wrapper coverage for Bluetooth, permissions, printer, and external storage APIs
  • register the module in package exports and docs status
  • add focused unit tests for request construction and namespace coverage

Tests

  • python -m pytest tests/test_core_external_device.py

@jmcte jmcte force-pushed the pr/core-external-device branch from 2d3afc9 to 4d27893 Compare April 17, 2026 12:46
@joeperpetua
Copy link
Copy Markdown
Collaborator

Are all these unit tests useful at all? What do they actually test? Just that the request fires? If no actual logic is being tested, then I don't see the purpose for there even being tests.

@N4S4
Copy link
Copy Markdown
Owner

N4S4 commented Apr 17, 2026

I agree with @joeperpetua

@jmcte jmcte force-pushed the pr/core-external-device branch from db2210d to bd8bd1f Compare April 18, 2026 21:32
@jmcte
Copy link
Copy Markdown
Contributor Author

jmcte commented Apr 18, 2026

Thanks, that was fair feedback. I updated the tests so they now assert request contracts instead of only checking that request_data was called. They validate the API namespace, path, version, DSM method, request parameters, optional parameter omission, and boolean conversion for representative wrapper methods. I made the same adjustment on the CoreNetwork PR because it had the same test pattern.

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.

3 participants