Skip to content

Commit 0c191de

Browse files
JasonXuDeveloperclaude
JasonXuDeveloper
and
claude
committed
fix(test): address code review feedback
- Convert sync [Test] to [UnityTest] with await for ExecuteAsync tests - Rename misleading test names to match assertions - Remove handler invocation tests with poor error handling - Use discard for intentionally unused task variables Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: JasonXuDeveloper - 傑 <jason@xgamedev.net>
1 parent 6709589 commit 0c191de

File tree

7 files changed

+64
-102
lines changed

7 files changed

+64
-102
lines changed

‎UnityProject/Packages/com.jasonxudeveloper.jengine.ui/Tests/Editor/Components/Form/JDropdownTests.cs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -320,15 +320,14 @@ public void Constructor_PopupFieldHasZeroMargins()
320320

321321
#endregion
322322

323-
#region Panel Attachment Tests
323+
#region Child Composition Tests
324324

325325
[Test]
326-
public void OnAttachToPanel_RegistersCallback()
326+
public void Constructor_HasSingleChild()
327327
{
328-
// Verify the callback is registered
329328
var dropdown = new JDropdown(_choices);
330329

331-
// The PopupField should be a child
330+
// The PopupField should be the only child
332331
Assert.AreEqual(1, dropdown.childCount);
333332
Assert.AreSame(dropdown.PopupField, dropdown.ElementAt(0));
334333
}

‎UnityProject/Packages/com.jasonxudeveloper.jengine.ui/Tests/Editor/Components/Form/JObjectFieldTests.cs‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -223,15 +223,14 @@ public void Constructor_InternalObjectFieldHasZeroMargins()
223223

224224
#endregion
225225

226-
#region Panel Attachment Tests
226+
#region Child Composition Tests
227227

228228
[Test]
229-
public void OnAttachToPanel_RegistersCallback()
229+
public void Constructor_HasSingleChild()
230230
{
231-
// Verify the callback is registered (element should be configured)
232231
var field = new JObjectField<GameObject>();
233232

234-
// The ObjectField should be a child
233+
// The ObjectField should be the only child
235234
Assert.AreEqual(1, field.childCount);
236235
Assert.AreSame(field.ObjectField, field.ElementAt(0));
237236
}
@@ -255,10 +254,9 @@ public void Constructor_AppliesInputContainerStyle()
255254
#region BindProperty Tests
256255

257256
[Test]
258-
public void BindProperty_NullProperty_DoesNotThrow()
257+
public void BindProperty_MethodExists()
259258
{
260-
// BindProperty with null should be handled gracefully by underlying field
261-
// Note: May throw depending on Unity version, but we verify the method exists
259+
// Verify the method exists and is accessible
262260
Assert.IsNotNull((System.Action<SerializedProperty>)_objectField.BindProperty);
263261
}
264262

‎UnityProject/Packages/com.jasonxudeveloper.jengine.ui/Tests/Editor/Components/Form/JTextFieldTests.cs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -265,15 +265,14 @@ public void Constructor_InternalTextFieldHasZeroMargins()
265265

266266
#endregion
267267

268-
#region Panel Attachment Tests
268+
#region Child Composition Tests
269269

270270
[Test]
271-
public void OnAttachToPanel_RegistersCallback()
271+
public void Constructor_HasSingleChild()
272272
{
273-
// Verify the callback is registered
274273
var field = new JTextField();
275274

276-
// The TextField should be a child
275+
// The TextField should be the only child
277276
Assert.AreEqual(1, field.childCount);
278277
Assert.AreSame(field.TextField, field.ElementAt(0));
279278
}

‎UnityProject/Packages/com.jasonxudeveloper.jengine.ui/Tests/Editor/Internal/EditorUIRegistrationTests.cs‎

Lines changed: 0 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -62,53 +62,5 @@ public void CreateInspectorHandler_PointsToBootstrapEditorUI()
6262
}
6363

6464
#endregion
65-
66-
#region Handler Invocation Tests
67-
68-
[Test]
69-
public void CreatePanelContentHandler_CanBeInvoked_WithNullParameters()
70-
{
71-
// Verify the handler can be called (may return null or throw gracefully)
72-
// This tests that the method reference is valid
73-
Assert.DoesNotThrow(() =>
74-
{
75-
try
76-
{
77-
Panel.CreatePanelContentHandler?.Invoke(null, null, null);
78-
}
79-
catch (System.NullReferenceException)
80-
{
81-
// Expected - we passed null parameters
82-
}
83-
catch (System.ArgumentNullException)
84-
{
85-
// Expected - we passed null parameters
86-
}
87-
});
88-
}
89-
90-
[Test]
91-
public void CreateInspectorHandler_CanBeInvoked_WithNullParameters()
92-
{
93-
// Verify the handler can be called (may return null or throw gracefully)
94-
// This tests that the method reference is valid
95-
Assert.DoesNotThrow(() =>
96-
{
97-
try
98-
{
99-
BootstrapEditor.CreateInspectorHandler?.Invoke(null, null);
100-
}
101-
catch (System.NullReferenceException)
102-
{
103-
// Expected - we passed null parameters
104-
}
105-
catch (System.ArgumentNullException)
106-
{
107-
// Expected - we passed null parameters
108-
}
109-
});
110-
}
111-
112-
#endregion
11365
}
11466
}

‎UnityProject/Packages/com.jasonxudeveloper.jengine.ui/Tests/Editor/MessageBoxTests.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -698,10 +698,10 @@ public IEnumerator Show_WithRealPrefab_ReusesPooledInstance() => UniTask.ToCorou
698698
[UnityTest]
699699
public IEnumerator CloseAll_WithRealPrefab_ClosesAllActive() => UniTask.ToCoroutine(async () =>
700700
{
701-
// Show multiple boxes without awaiting
702-
var task1 = MessageBox.Show("Test1", "Content1");
701+
// Show multiple boxes without awaiting (use discard to suppress unused variable warning)
702+
_ = MessageBox.Show("Test1", "Content1");
703703
await UniTask.Yield();
704-
var task2 = MessageBox.Show("Test2", "Content2");
704+
_ = MessageBox.Show("Test2", "Content2");
705705
await UniTask.Yield();
706706

707707
Assert.AreEqual(2, MessageBox.ActiveCount);

‎UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Editor/JActionAwaiterTests.cs‎

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,23 +26,25 @@ public void TearDown()
2626
JAction.ClearPool();
2727
}
2828

29-
[Test]
30-
public void ExecuteAsync_ReturnsHandle()
29+
[UnityTest]
30+
public IEnumerator ExecuteAsync_ReturnsHandle() => UniTask.ToCoroutine(async () =>
3131
{
3232
using var action = JAction.Create().Do(() => { });
3333
var handle = action.ExecuteAsync();
3434

3535
Assert.IsNotNull(handle.Action);
36-
}
36+
await handle;
37+
});
3738

38-
[Test]
39-
public void ExecuteAsync_HandleHasCorrectAction()
39+
[UnityTest]
40+
public IEnumerator ExecuteAsync_HandleHasCorrectAction() => UniTask.ToCoroutine(async () =>
4041
{
4142
using var action = JAction.Create().Do(() => { });
4243
var handle = action.ExecuteAsync();
4344

4445
Assert.AreSame(action, handle.Action);
45-
}
46+
await handle;
47+
});
4648

4749
[UnityTest]
4850
public IEnumerator ExecuteAsync_CanBeAwaited() => UniTask.ToCoroutine(async () =>
@@ -96,16 +98,17 @@ public void TearDown()
9698

9799
#region GetAwaiter Tests
98100

99-
[Test]
100-
public void GetAwaiter_ReturnsAwaiterInstance()
101+
[UnityTest]
102+
public IEnumerator GetAwaiter_ReturnsAwaiterInstance() => UniTask.ToCoroutine(async () =>
101103
{
102104
using var action = JAction.Create().Do(() => { });
103105
var handle = action.ExecuteAsync();
104106

105107
var awaiter = handle.GetAwaiter();
106108

107109
Assert.IsInstanceOf<JActionExecutionAwaiter>(awaiter);
108-
}
110+
await handle;
111+
});
109112

110113
#endregion
111114

@@ -221,15 +224,16 @@ public IEnumerator OnCompleted_DuringExecution_RegistersCallback() => UniTask.To
221224
Assert.IsTrue(invoked);
222225
});
223226

224-
[Test]
225-
public void OnCompleted_NullContinuation_HandlesGracefully()
227+
[UnityTest]
228+
public IEnumerator OnCompleted_NullContinuation_HandlesGracefully() => UniTask.ToCoroutine(async () =>
226229
{
227230
using var action = JAction.Create().Do(() => { });
228231
var handle = action.ExecuteAsync();
229232
var awaiter = handle.GetAwaiter();
230233

231234
Assert.DoesNotThrow(() => awaiter.OnCompleted(null));
232-
}
235+
await handle;
236+
});
233237

234238
#endregion
235239

@@ -252,15 +256,16 @@ public IEnumerator UnsafeOnCompleted_AfterCompletion_InvokesContinuationImmediat
252256
Assert.IsTrue(invoked);
253257
});
254258

255-
[Test]
256-
public void UnsafeOnCompleted_NullContinuation_HandlesGracefully()
259+
[UnityTest]
260+
public IEnumerator UnsafeOnCompleted_NullContinuation_HandlesGracefully() => UniTask.ToCoroutine(async () =>
257261
{
258262
using var action = JAction.Create().Do(() => { });
259263
var handle = action.ExecuteAsync();
260264
var awaiter = handle.GetAwaiter();
261265

262266
Assert.DoesNotThrow(() => awaiter.UnsafeOnCompleted(null));
263-
}
267+
await handle;
268+
});
264269

265270
#endregion
266271

‎UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Editor/JActionExecutionHandleTests.cs‎

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -29,25 +29,28 @@ public void TearDown()
2929

3030
#region Action Property Tests
3131

32-
[Test]
33-
public void Action_ReturnsAssociatedJAction()
32+
[UnityTest]
33+
public IEnumerator Action_ReturnsAssociatedJAction() => UniTask.ToCoroutine(async () =>
3434
{
3535
using var action = JAction.Create().Do(() => { });
3636
var handle = action.ExecuteAsync();
3737

3838
Assert.AreSame(action, handle.Action);
39-
}
39+
await handle;
40+
});
4041

41-
[Test]
42-
public void Action_MultipleHandles_ReturnSameAction()
42+
[UnityTest]
43+
public IEnumerator Action_MultipleHandles_ReturnSameAction() => UniTask.ToCoroutine(async () =>
4344
{
4445
using var action = JAction.Create().Parallel().Do(() => { });
4546
var handle1 = action.ExecuteAsync();
4647
var handle2 = action.ExecuteAsync();
4748

4849
Assert.AreSame(action, handle1.Action);
4950
Assert.AreSame(action, handle2.Action);
50-
}
51+
52+
await UniTask.WhenAll(handle1.AsUniTask(), handle2.AsUniTask());
53+
});
5154

5255
#endregion
5356

@@ -154,8 +157,8 @@ public IEnumerator Cancel_CompletedHandle_DoesNotThrow() => UniTask.ToCoroutine(
154157
Assert.DoesNotThrow(() => handle.Cancel());
155158
});
156159

157-
[Test]
158-
public void Cancel_MultipleTimes_DoesNotThrow()
160+
[UnityTest]
161+
public IEnumerator Cancel_MultipleTimes_DoesNotThrow() => UniTask.ToCoroutine(async () =>
159162
{
160163
using var action = JAction.Create().Delay(10f);
161164
var handle = action.ExecuteAsync();
@@ -166,7 +169,9 @@ public void Cancel_MultipleTimes_DoesNotThrow()
166169
handle.Cancel();
167170
handle.Cancel();
168171
});
169-
}
172+
173+
await handle;
174+
});
170175

171176
[UnityTest]
172177
public IEnumerator Cancel_StopsExecution() => UniTask.ToCoroutine(async () =>
@@ -203,16 +208,17 @@ public IEnumerator Cancel_InvokesOnCancelCallback() => UniTask.ToCoroutine(async
203208

204209
#region GetAwaiter Tests
205210

206-
[Test]
207-
public void GetAwaiter_ReturnsAwaiter()
211+
[UnityTest]
212+
public IEnumerator GetAwaiter_ReturnsAwaiter() => UniTask.ToCoroutine(async () =>
208213
{
209214
using var action = JAction.Create().Do(() => { });
210215
var handle = action.ExecuteAsync();
211216

212217
var awaiter = handle.GetAwaiter();
213218

214219
Assert.IsInstanceOf<JActionExecutionAwaiter>(awaiter);
215-
}
220+
await handle;
221+
});
216222

217223
[UnityTest]
218224
public IEnumerator GetAwaiter_AfterCompletion_ReturnsCompletedAwaiter() => UniTask.ToCoroutine(async () =>
@@ -270,8 +276,8 @@ public IEnumerator Dispose_MultipleTimes_DoesNotThrow() => UniTask.ToCoroutine(a
270276

271277
#region Implicit Operator Tests
272278

273-
[Test]
274-
public void ImplicitOperator_ConvertsToJAction()
279+
[UnityTest]
280+
public IEnumerator ImplicitOperator_ConvertsToJAction() => UniTask.ToCoroutine(async () =>
275281
{
276282
var action = JAction.Create().Do(() => { });
277283
var handle = action.ExecuteAsync();
@@ -280,8 +286,9 @@ public void ImplicitOperator_ConvertsToJAction()
280286

281287
Assert.AreSame(action, converted);
282288

289+
await handle;
283290
action.Dispose();
284-
}
291+
});
285292

286293
[Test]
287294
public void ImplicitOperator_DefaultHandle_ReturnsNull()
@@ -433,15 +440,16 @@ public IEnumerator OnCompleted_AfterCompletion_InvokesContinuationImmediately()
433440
Assert.IsTrue(invoked);
434441
});
435442

436-
[Test]
437-
public void OnCompleted_NullContinuation_HandlesGracefully()
443+
[UnityTest]
444+
public IEnumerator OnCompleted_NullContinuation_HandlesGracefully() => UniTask.ToCoroutine(async () =>
438445
{
439446
using var action = JAction.Create().Do(() => { });
440447
var handle = action.ExecuteAsync();
441448
var awaiter = handle.GetAwaiter();
442449

443450
Assert.DoesNotThrow(() => awaiter.OnCompleted(null));
444-
}
451+
await handle;
452+
});
445453

446454
#endregion
447455

@@ -462,15 +470,16 @@ public IEnumerator UnsafeOnCompleted_AfterCompletion_InvokesContinuationImmediat
462470
Assert.IsTrue(invoked);
463471
});
464472

465-
[Test]
466-
public void UnsafeOnCompleted_NullContinuation_HandlesGracefully()
473+
[UnityTest]
474+
public IEnumerator UnsafeOnCompleted_NullContinuation_HandlesGracefully() => UniTask.ToCoroutine(async () =>
467475
{
468476
using var action = JAction.Create().Do(() => { });
469477
var handle = action.ExecuteAsync();
470478
var awaiter = handle.GetAwaiter();
471479

472480
Assert.DoesNotThrow(() => awaiter.UnsafeOnCompleted(null));
473-
}
481+
await handle;
482+
});
474483

475484
#endregion
476485
}

0 commit comments

Comments
 (0)