Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jul 1, 2025

This PR implements comprehensive automated tests for the two CopyToTemporaryStreamAsync extension method overloads as requested in the issue. The tests follow Sociable Unit Testing principles, testing real components in combination without test doubles.

🧪 Tests Added

8 comprehensive tests covering both overloads:

Basic CopyToTemporaryStreamAsync overload:

  • ✅ MemoryStream scenario with small data (< 80KB)
  • ✅ FileStream scenario with large data (> 80KB)
  • ✅ Custom buffer size parameter
  • ✅ Custom options parameter

Plugin-enabled CopyToTemporaryStreamAsync overload:

  • ✅ HashingPlugin with SHA-1 algorithm (MemoryStream scenario)
  • ✅ HashingPlugin with SHA-1 algorithm (FileStream scenario)
  • ✅ Multiple hash algorithms (SHA-1 + SHA-256)
  • ✅ Custom buffer size with plugins

All tests verify:

  • Correct stream type selection (Memory vs File based on 80KB threshold)
  • Accurate data copying and content verification
  • Hash calculation correctness using SHA-1 as specified
  • Parameter handling (buffer sizes, options, file paths)

🐛 Critical Bugs Fixed

During test development, two critical bugs were discovered and fixed in the existing codebase:

1. Plugin Setup Bug in TemporaryStreamServiceExtensions

Issue: Line 126 was incorrectly passing the source stream to plugin setup instead of the outermostStream (destination).

// Before (incorrect)
outermostStream = await plugins[i].SetUpAsync(source, cancellationToken).ConfigureAwait(false);

// After (correct)  
outermostStream = await plugins[i].SetUpAsync(outermostStream, cancellationToken).ConfigureAwait(false);

This caused plugins to wrap the wrong stream, leading to NotSupportedException: Memory stream is not expandable.

2. Multiple Hash Calculator Bug in HashingPlugin

Issue: The plugin wasn't properly chaining multiple hash calculators - all calculators were wrapping the same inner stream instead of creating a chain.

// Before (incorrect)
for (var i = 0; i < HashCalculators.Length; i++)
{
    outermostStream = HashCalculators[i].CreateWrappingCryptoStream(innerStream, leaveWrappedStreamOpen: true);
}

// After (correct)
Stream currentStream = innerStream;
for (var i = 0; i < HashCalculators.Length; i++)
{
    currentStream = HashCalculators[i].CreateWrappingCryptoStream(currentStream, leaveWrappedStreamOpen: true);
}

📁 Test Resources

Added sample PDF files as requested:

  • less-than-80KB.pdf (596 bytes) - triggers MemoryStream usage
  • more-than-80KB.pdf (80,602 bytes) - triggers FileStream usage

✅ Results

  • All 103 tests pass (95 existing + 8 new)
  • No breaking changes to existing functionality
  • Comprehensive coverage of both extension method overloads
  • Real-world demonstration with actual file size thresholds

Fixes #6.


💡 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 AI and others added 3 commits July 1, 2025 19:42
Co-authored-by: feO2x <7884158+feO2x@users.noreply.github.com>
…s and fix plugin bugs

Co-authored-by: feO2x <7884158+feO2x@users.noreply.github.com>
Co-authored-by: feO2x <7884158+feO2x@users.noreply.github.com>
Copilot AI changed the title [WIP] Automated tests for CopyToTemporaryStreamAsync overloads Add comprehensive tests for CopyToTemporaryStreamAsync overloads and fix plugin bugs Jul 1, 2025
Copilot AI requested a review from feO2x July 1, 2025 20:11
Copilot finished work on behalf of feO2x July 1, 2025 20:11
feO2x added 10 commits July 5, 2025 22:46
Signed-off-by: Kenny Pflug <kenny.pflug@live.de>
Signed-off-by: Kenny Pflug <kenny.pflug@live.de>
Signed-off-by: Kenny Pflug <kenny.pflug@live.de>
Signed-off-by: Kenny Pflug <kenny.pflug@live.de>
Signed-off-by: Kenny Pflug <kenny.pflug@live.de>
Signed-off-by: Kenny Pflug <kenny.pflug@live.de>
Signed-off-by: Kenny Pflug <kenny.pflug@live.de>
Signed-off-by: Kenny Pflug <kenny.pflug@live.de>
…reamService interface

Signed-off-by: Kenny Pflug <kenny.pflug@live.de>
Signed-off-by: Kenny Pflug <kenny.pflug@live.de>
Copy link
Owner

@feO2x feO2x left a comment

Choose a reason for hiding this comment

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

LGTM - I fixed some things, but overall great work

@feO2x feO2x marked this pull request as ready for review July 5, 2025 23:17
@feO2x feO2x merged commit a9f40c4 into main Jul 5, 2025
1 check passed
@feO2x feO2x deleted the copilot/fix-6 branch July 5, 2025 23:19
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.

Automated tests for CopyToTemporaryStreamAsync overloads

2 participants