Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 4, 2025

Replace collection equality assertions with CollectionAssert.AreEqual

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

Replace Assert.IsTrue(xxx.IsEqualTo(yyy)) and Assert.IsTrue(xxx.SequenceEqual(yyy))
with CollectionAssert.AreEqual(expected, actual) across 55 test files

Co-authored-by: Rob-Hague <5132141+Rob-Hague@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor collection assertions to use CollectionAssert.AreEqual Replace Assert.IsTrue with CollectionAssert.AreEqual for collection comparisons Nov 4, 2025
Copilot AI requested a review from Rob-Hague November 4, 2025 07:54
Copy link
Collaborator

@Rob-Hague Rob-Hague left a comment

Choose a reason for hiding this comment

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

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

…y using directives

- Fixed argument order in KeyExchangeDhGroupExchangeReplyTest.cs (expected first, actual second)
- Fixed argument order in KeyExchangeInitMessageTest.cs (expected first, actual second)
- Removed unnecessary 'using System.Linq' directives from all affected test files

Co-authored-by: Rob-Hague <5132141+Rob-Hague@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 13, 2025

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

Fixed in commit 2821526. 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 AI requested a review from Rob-Hague November 13, 2025 20:35
Copilot finished work on behalf of Rob-Hague November 13, 2025 20:35
@Rob-Hague Rob-Hague marked this pull request as ready for review November 13, 2025 21:31
@Rob-Hague
Copy link
Collaborator

@copilot resolve conflicts

Copy link
Contributor Author

Copilot AI commented Nov 14, 2025

@copilot resolve conflicts

Conflicts resolved in commit 8e5e1bd. 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 AI requested a review from Rob-Hague November 14, 2025 21:19
Copilot finished work on behalf of Rob-Hague November 14, 2025 21:19
…-collection-assertions

# Conflicts:
#	test/Renci.SshNet.Tests/Classes/ShellStreamTest_Write_WriteBufferNotEmptyAndWriteLessBytesThanBufferCanContain.cs
#	test/Renci.SshNet.Tests/Classes/ShellStreamTest_Write_WriteBufferNotEmptyAndWriteMoreBytesThanBufferCanContain.cs
@Rob-Hague Rob-Hague force-pushed the copilot/refactor-collection-assertions branch from 8e5e1bd to b8a5836 Compare November 14, 2025 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants