[tests] Migrate Cecil tests to NUnit v4.#24525
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the Cecil test suite from NUnit v3 to NUnit v4, updating package references and converting assertion syntax to be compatible with the newer version.
Changes:
- Updated NUnit and related test package versions in
Directory.Build.props - Replaced hardcoded package versions with centralized version variables in
cecil-tests.csproj - Converted legacy NUnit assertion syntax to constraint model (e.g.,
Assert.False→Assert.That(..., Is.False))
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Directory.Build.props | Defines centralized version variables for NUnit v4 packages and test SDK |
| tests/cecil-tests/cecil-tests.csproj | Replaces hardcoded package versions with centralized variables |
| tests/cecil-tests/Test.cs | Updates assertion syntax from legacy to constraint model |
| tests/cecil-tests/Helper.cs | Updates assertion syntax and adds null-coalescing operator for null safety |
| tests/cecil-tests/GetterExceptionTest.cs | Converts Assert.AreEqual to constraint model |
| tests/cecil-tests/GenericPInvokes.cs | Converts assertions to constraint model |
| tests/cecil-tests/BlittablePInvokes.cs | Converts Assert.IsNotNull to constraint model |
| tests/cecil-tests/ApiCapitalizationTest.cs | Converts Assert.AreEqual to constraint model |
| tests/cecil-tests/ApiAvailabilityTest.cs | Converts Assert.AreEqual to constraint model |
tests/cecil-tests/GenericPInvokes.cs
Outdated
| var assembly = info.Assembly; | ||
| var pinvokes = AllPInvokes (assembly).Where (IsPInvokeOK); | ||
| Assert.IsTrue (pinvokes.Count () > 0); | ||
| Assert.That (pinvokes.Count, Is.GreaterThan (0), Is.True); |
There was a problem hiding this comment.
The third argument Is.True appears to be incorrectly used. The constraint Is.GreaterThan(0) already expresses the assertion; adding Is.True is unnecessary and likely invalid syntax. Remove the Is.True parameter.
| Assert.That (pinvokes.Count, Is.GreaterThan (0), Is.True); | |
| Assert.That (pinvokes.Count, Is.GreaterThan (0)); |
tests/cecil-tests/GenericPInvokes.cs
Outdated
| var assembly = info.Assembly; | ||
| var pinvokes = AllPInvokes (assembly).Where (IsPInvokeOK); | ||
| Assert.IsTrue (pinvokes.Count () > 0); | ||
| Assert.That (pinvokes.Count, Is.GreaterThan (0), Is.True); |
There was a problem hiding this comment.
Using Count property instead of Count() method would be more appropriate if pinvokes is an ICollection or IList. If it's an IEnumerable, the call is correct, but the type should be clarified for consistency with line 38-39.
| Assert.That (pinvokes.Count, Is.GreaterThan (0), Is.True); | |
| Assert.That (pinvokes.Count (), Is.GreaterThan (0), Is.True); |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #d66dfd3] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ [CI Build #d66dfd3] Build passed (Build packages) ✅Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
✅ [CI Build #d66dfd3] Build passed (Build macOS tests) ✅Pipeline on Agent |
💻 [CI Build #d66dfd3] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #d66dfd3] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #d66dfd3] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
💻 [CI Build #d66dfd3] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
💻 [CI Build #d66dfd3] Tests on macOS arm64 - Mac Tahoe (26) passed 💻✅ All tests on macOS arm64 - Mac Tahoe (26) passed. Pipeline on Agent |
🚀 [CI Build #d66dfd3] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 125 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
No description provided.