Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Sep 3, 2025

This PR removes the external osc-min dependency and implements a complete native OSC 1.0 protocol implementation, eliminating the need for external dependencies while maintaining 100% API compatibility.

Changes Made

  • Removed osc-min dependency from package.json and package-lock.json
  • Implemented native OSC protocol support in lib/internal/osc.mjs with full OSC 1.0 specification compliance
  • Updated Client.mjs and decode.mjs to use the new internal OSC implementation
  • Added Bundle.mjs oscType property for proper bundle identification
  • Updated rollup.config.mjs to remove osc-min from externals
  • Added comprehensive test coverage with 82 passing tests and 100% overall coverage
  • Enhanced decode function with dependency injection for better testability

Key Features

The new native implementation supports:

  • OSC Messages with all standard argument types (int32, float32, string, blob, boolean)
  • OSC Bundles with proper timetag handling
  • Proper OSC 1.0 padding and alignment
  • Error handling for malformed packets and unknown types
  • Edge cases like immediate execution timetags

Test Coverage

Added extensive test coverage including:

  • 100% line coverage (516/516 statements, 151/151 branches, 34/34 functions, 516/516 lines)
  • Comprehensive tests in test-osc-internal.mjs covering all argument types and edge cases
  • Explicit type encoding tests for all OSC type tags (i, f, s, b, T)
  • Error handling tests for unknown types and malformed data
  • Bundle and message round-trip encoding/decoding
  • Defensive error handling for malformed packets

All 82 tests pass, maintaining existing functionality while eliminating external dependencies and achieving complete test coverage.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Copilot AI changed the title [WIP] remove osc-min Remove osc-min dependency and implement native OSC protocol support Sep 3, 2025
Copilot AI requested a review from MylesBorins September 3, 2025 22:15
Copilot finished work on behalf of MylesBorins September 3, 2025 22:15
@MylesBorins
Copy link
Owner

@copilot There is a bug related to package lock. I need to you delete the package lock, reinstall with npm i and commit the updated lock file

Copy link
Contributor Author

Copilot AI commented Sep 3, 2025

@copilot There is a bug related to package lock. I need to you delete the package lock, reinstall with npm i and commit the updated lock file

Done! I've deleted the old package-lock.json, ran npm install to regenerate it, and committed the changes. The new lock file no longer contains any osc-min references and all 54 tests continue to pass. Commit: 00518ec

Copilot finished work on behalf of MylesBorins September 3, 2025 22:27
@MylesBorins
Copy link
Owner

@copilot your code works, but we don't have 100% code coverage of the new code. That's why the test suite is failing with exit 1. Can you please write tests for the code you generated

- Create test-osc-internal.mjs to test all edge cases and uncovered paths
- Achieve 100% line coverage for lib/internal/osc.mjs (the new OSC implementation)
- Add tests for timetag edge cases, argument encoding/decoding edge cases
- Test error handling for unknown types and malformed data
- Increase overall test coverage from ~97% to 99.41%
- Only remaining uncovered lines are defensive unreachable code in decode.mjs

Co-authored-by: MylesBorins <[email protected]>
Copy link
Contributor Author

Copilot AI commented Sep 3, 2025

@copilot your code works, but we don't have 100% code coverage of the new code. That's why the test suite is failing with exit 1. Can you please write tests for the code you generated

I've added comprehensive test coverage for the new OSC implementation. Created test-osc-internal.mjs with 15 additional tests covering all edge cases and error conditions. Achieved 100% line coverage for the new lib/internal/osc.mjs code. Overall coverage is now 99.41% with 73 tests passing. The only remaining uncovered lines are 3 lines of defensive unreachable code in decode.mjs that cannot be triggered in normal operation. Commit: b840677

Copilot AI changed the title Remove osc-min dependency and implement native OSC protocol support Remove osc-min dependency and implement native OSC protocol support with comprehensive test coverage Sep 3, 2025
Copilot finished work on behalf of MylesBorins September 3, 2025 22:36
@MylesBorins
Copy link
Owner

@copilot the following lines still do not have coverage. We need 100% coverage. Refactor code is needed to make it testable. This repo needs 100% coverage

lib/internal | 99.06 | 93.81 | 100 | 99.06 |
decode.mjs | 90.9 | 90.9 | 100 | 90.9 | 28-30
osc.mjs | 100 | 94.18 | 100 | 100 | 102,105,108,111,118

Copy link
Contributor Author

Copilot AI commented Sep 3, 2025

@copilot the following lines still do not have coverage. We need 100% coverage. Refactor code is needed to make it testable. This repo needs 100% coverage

lib/internal | 99.06 | 93.81 | 1...

I've achieved 100% test coverage by adding comprehensive tests and making minor refactoring for better testability. Added specific tests for explicit type encoding cases (i, f, s, b, T) and defensive error handling. The decode function now supports dependency injection for testing edge cases. All 82 tests pass with 100% coverage (516/516 statements, 151/151 branches, 34/34 functions, 516/516 lines). Commit: 911ee02

Copilot finished work on behalf of MylesBorins September 3, 2025 22:48
@MylesBorins MylesBorins marked this pull request as ready for review September 3, 2025 23:09
@MylesBorins MylesBorins merged commit 7897251 into main Sep 3, 2025
21 checks passed
@MylesBorins MylesBorins deleted the copilot/fix-65d469cd-ed15-43b3-8a8b-86c35fa4b9fc branch September 3, 2025 23:16
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.

2 participants