Skip to content

Conversation

@JasonXuDeveloper
Copy link
Owner

@JasonXuDeveloper JasonXuDeveloper commented Jan 30, 2026

Summary

  • Fixes CodeQL security alert #12 (cs/dispose-not-called-on-throw)
  • Changes var action to using var action to ensure disposal on exception

Problem

The Reset_AllowsReuse test created a JAction and called Dispose() at the end. However, if Execute(), Reset(), or Do() threw an exception, the method would exit early and Dispose() would never be called.

Solution

Use a using declaration which guarantees disposal even when exceptions occur.

Test plan

  • CI tests pass

🤖 Generated with Claude Code

Fixes CodeQL alert cs/dispose-not-called-on-throw. If Execute(),
Reset(), or Do() threw an exception, Dispose() would not be called.

Using a using declaration ensures the JAction is disposed even if
an exception occurs.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: JasonXuDeveloper - 傑 <jason@xgamedev.net>
Copilot AI review requested due to automatic review settings January 30, 2026 22:39
@JasonXuDeveloper JasonXuDeveloper enabled auto-merge (squash) January 30, 2026 22:39
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates a unit test to ensure a JAction instance is always disposed, even if an exception occurs during execution, addressing a CodeQL cs/dispose-not-called-on-throw alert.

Changes:

  • Replace var action = JAction.Create() with using var action = JAction.Create() to guarantee disposal on all code paths.
  • Remove the now-redundant explicit action.Dispose() call at the end of the test.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-approved: Copilot review found no issues and Unity Tests passed (or were skipped for non-code changes).

@github-actions
Copy link

Unity Test Results

EditMode: All tests passed
PlayMode: All tests passed

Unity Version: 2022.3.55f1
Project Path: UnityProject

✅ All tests passed! The PR is ready for review.

View workflow run

Click here to view the full workflow run

@JasonXuDeveloper JasonXuDeveloper merged commit 05a92f8 into master Jan 30, 2026
24 checks passed
@JasonXuDeveloper JasonXuDeveloper deleted the fix/jaction-test-dispose-on-exception branch January 30, 2026 22:44
@codecov
Copy link

codecov bot commented Jan 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.01%. Comparing base (bc82092) to head (749a95f).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #611       +/-   ##
===========================================
+ Coverage   42.84%   85.01%   +42.17%     
===========================================
  Files          38       60       +22     
  Lines        4250     7683     +3433     
===========================================
+ Hits         1821     6532     +4711     
+ Misses       2429     1151     -1278     
Flag Coverage Δ
ui 83.60% <ø> (+69.73%) ⬆️
util 90.36% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants