Skip to content

Refactor synthutils export structure to properly support Jest tests and coverage #5820

Open
stutijain2006 wants to merge 3 commits intosugarlabs:masterfrom
stutijain2006:MacrosTest
Open

Refactor synthutils export structure to properly support Jest tests and coverage #5820
stutijain2006 wants to merge 3 commits intosugarlabs:masterfrom
stutijain2006:MacrosTest

Conversation

@stutijain2006
Copy link
Contributor

This PR fixes the export structure of synthutils.js so that Jest tests run correctly and coverage is properly recorded.

Earlier, although test cases were defined and running, the global test coverage was showing 0%, which indicated that functions were not being properly linked to the test environment. Some functions were also failing due to incorrect this binding and instance access.

Problem -

  • Functions inside Synth were defined as instance methods.
  • But they were being exported in a way that lost the correct this context.
  • Tests were referencing class methods, while the actual logic depended on instance-level state.
  • Some internal functions (like _createSampleSynth, setupRecorder, getTunerFrequency) were not properly accessible.
  • Coverage report was almost 0% globally despite having defined test cases.

What was done-

  1. Created a single Synth instance for export
    const synthInstance = new Synth();
    All exported functions are now bound to this instance:
loadSamples: synthInstance.loadSamples.bind(synthInstance),
getTunerFrequency: synthInstance.getTunerFrequency.bind(synthInstance),
...
  1. Exported synthInstance for test control
    synthInstance.detectPitch = jest.fn(...)
    instead of modifying the classes directly.
  2. Added required global mocks in test setup
    Some global helpers used inside synthutils were not defined in the test environment.
    Mocks were added to stabilize execution.

Final Result-

  • All 86 test cases pass
  • No runtime logic changed
  • Proper instance binding
  • Coverage is now correctly measured
  • Global coverage is no longer showing near 0%
  • synthutils coverage improved significantly

Importance of this PR-
Before-

  • Tests existed
  • But universal test was not importing this and showing 0%
  • Some internal logic was not actually being validated correctly

After-

  • Tests truly execute the intended logic
  • Coverage metrics are accurate
  • The module structure is cleaner and more maintainable
Screenshot 2026-02-19 231105 Screenshot 2026-02-19 231218

@github-actions
Copy link
Contributor

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

@vanshika2720
Copy link
Contributor

@stutijain2006 please run prettier on modified files.

@github-actions
Copy link
Contributor

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

@kartikktripathi
Copy link
Contributor

Try rebasing this, I believe you ran prettier on synthutils.js too but it still shows formatting problems, maybe rebasing will work.

@github-actions
Copy link
Contributor

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

@stutijain2006
Copy link
Contributor Author

@omsuneri This PR increases the test coverage for synthutils and I believe this is ready to merged. Please see to it.

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