Skip to content

fix: handle null turtle in EnsembleBlocks._blockFindTurtle#7736

Open
Jetshree wants to merge 1 commit into
sugarlabs:masterfrom
Jetshree:fix/ensemble-null-pointer
Open

fix: handle null turtle in EnsembleBlocks._blockFindTurtle#7736
Jetshree wants to merge 1 commit into
sugarlabs:masterfrom
Jetshree:fix/ensemble-null-pointer

Conversation

@Jetshree

@Jetshree Jetshree commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes null pointer handling and improves error messaging in ensemble compositions.

What Changed

  • Added error message display in _blockFindTurtle() when target turtle not found
  • Prevents silent failures and provides user feedback
  • Added 6 comprehensive test cases for null turtle scenarios

Testing

All 159 tests pass (6 new + 153 existing)
98.23% code coverage on EnsembleBlocks.js

Why It Matters

When users try to control an ensemble with a non-existent turtle name, the app now shows an error message instead of silently failing. This is critical for multi-turtle compositions stability.

PR Category

  • Bug fix

When getTargetTurtle() returns null (turtle name not found), show user error
message instead of returning null without notification.

Added comprehensive test coverage for _blockFindTurtle():
- Null block handling
- Missing turtle name connection
- Non-existent turtle name
- Successful turtle lookup

Fixes crash in multi-turtle ensemble compositions when target turtle not found.
@github-actions github-actions Bot added bug fix Fixes a bug or incorrect behavior size/M Medium: 50-249 lines changed area/javascript Changes to JS source files area/tests Changes to test files labels Jul 5, 2026
@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.99%. Comparing base (d757018) to head (da4c37b).
⚠️ Report is 61 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7736   +/-   ##
=======================================
  Coverage   51.99%   51.99%           
=======================================
  Files         170      170           
  Lines       56865    56866    +1     
=======================================
+ Hits        29567    29569    +2     
+ Misses      27298    27297    -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@walterbender

Copy link
Copy Markdown
Member

Looks good - having a user-facing message makes sense. Just need to wait until after string freeze to merge.

@Jetshree

Jetshree commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

Looks good - having a user-facing message makes sense. Just need to wait until after string freeze to merge.

Thanks for review : )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/javascript Changes to JS source files area/tests Changes to test files bug fix Fixes a bug or incorrect behavior size/M Medium: 50-249 lines changed

Projects

Development

Successfully merging this pull request may close these issues.

2 participants