SSH.NET icon indicating copy to clipboard operation
SSH.NET copied to clipboard

Replace Assert.IsTrue with CollectionAssert.AreEqual for collection comparisons

Open Copilot opened this issue 4 months ago • 3 comments

Replace collection equality assertions with CollectionAssert.AreEqual

  • [x] Find all test files with Assert.IsTrue(xxx.IsEqualTo(yyy)) or Assert.IsTrue(xxx.SequenceEqual(yyy)) patterns (55 files, 96 occurrences)
  • [x] Replace with CollectionAssert.AreEqual(expected, actual) with correct argument order
  • [x] Handle complex patterns including method calls, property access, and .Take()/.Skip() operations
  • [x] Add .ToArray() where needed for IEnumerable results from LINQ methods
  • [x] Fix argument order issues identified in code review (KeyExchangeDhGroupExchangeReplyTest and KeyExchangeInitMessageTest)
  • [x] Remove unnecessary using directives (IDE0005 warnings)
  • [x] Resolve merge conflicts with develop branch
  • [x] All changes compile successfully with 0 warnings and 0 errors

Summary

Successfully refactored 96 collection equality assertions across 55 test files:

  • Replaced Assert.IsTrue(xxx.IsEqualTo(yyy)) with CollectionAssert.AreEqual(expected, actual)
  • Replaced Assert.IsTrue(xxx.SequenceEqual(yyy)) with CollectionAssert.AreEqual(expected, actual)
  • Fixed argument order to ensure expected value is first, actual value is second
  • Handled complex cases with method calls, property access, and LINQ operations
  • Added .ToArray() conversions for IEnumerable results from LINQ methods like Skip()
  • Removed unnecessary using System.Linq directives

Changes from review feedback

  • Fixed argument order in KeyExchangeDhGroupExchangeReplyTest.cs (lines 26-28): constant arrays now come first as expected values
  • Fixed argument order in KeyExchangeInitMessageTest.cs (lines 28-37): constant arrays now come first as expected values
  • Removed all unnecessary using directives flagged by IDE0005

Merge conflicts resolved

Merged with develop branch and resolved conflicts in:

  • ShellStreamTest_Write_WriteBufferNotEmptyAndWriteLessBytesThanBufferCanContain.cs
  • ShellStreamTest_Write_WriteBufferNotEmptyAndWriteMoreBytesThanBufferCanContain.cs

Conflict resolution strategy:

  • Adopted Assert.HasCount from develop (new API for checking collection length)
  • Kept CollectionAssert.AreEqual for collection comparisons (our refactoring goal)
Original prompt

find all places in tests where some collections are being asserted for equality, e.g. Assert.IsTrue(xxx.IsEqualTo(yyy)) or Assert.IsTrue(xxx.SequenceEqual(yyy)) and replace them with CollectionAssert.AreEqual. Pay attention to the order of arguments to CollectionAssert.AreEqual - the expected value is the first argument and the actual value is the second. If there is a separate assertion on the collections' lengths, then remove it since CollectionsAssert.AreEqual takes care of that as well


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot avatar Nov 04 '25 07:11 Copilot

@copilot there are multiple build errors of the form "Using directive is unnecessary", and some review comments to address:

Fixed in commit 2821526a. All IDE0005 warnings have been resolved by removing unnecessary using directives, and the argument order issues in the review comments have been corrected. The build now succeeds with 0 warnings and 0 errors.

Copilot avatar Nov 13 '25 20:11 Copilot

@copilot resolve conflicts

Rob-Hague avatar Nov 14 '25 21:11 Rob-Hague

@copilot resolve conflicts

Conflicts resolved in commit 8e5e1bdc. Merged develop branch and resolved conflicts in 2 test files by adopting Assert.HasCount from develop while keeping CollectionAssert.AreEqual for collection comparisons. Build succeeds with 0 warnings and 0 errors.

Copilot avatar Nov 14 '25 21:11 Copilot