Skip to content

Address code review feedback for large payload tests#30

Closed
Copilot wants to merge 2 commits intopayload-validation-tests-10150719905609723466from
copilot/sub-pr-29
Closed

Address code review feedback for large payload tests#30
Copilot wants to merge 2 commits intopayload-validation-tests-10150719905609723466from
copilot/sub-pr-29

Conversation

Copy link

Copilot AI commented Feb 16, 2026

Addresses code review feedback on the large payload validation tests PR to fix build warnings, improve parallelization behavior, and align with project conventions.

Changes:

  • Security: Replace SHA1 with SHA256 to avoid CA5350 analyzer warnings with TreatWarningsAsErrors
  • Parallelization: Add [DoNotParallelize] to prevent OOM/timeout when 500MB+ tests run concurrently under 4-worker parallel execution
  • Timeouts: Add [Timeout] attributes to all test methods (5min for ≤100MB, 10min for ≥500MB)
  • Code cleanup:
    • Fix namespace to tests.Tests matching other test files
    • Make RandomStream internal sealed per test assembly conventions
    • Remove unused imports (System.Text, Sisk.Cadente)
    • Use ListeningPort.GetRandomPort() instead of custom TCP listener logic
    • Remove redundant Sisk.Cadente project reference (available transitively)
// Before
namespace tests;
public class RandomStream : Stream { }

[TestMethod]
public async Task TestPayload_10MB() { }

// After  
namespace tests.Tests;
internal sealed class RandomStream : Stream { }

[TestMethod]
[Timeout(300000)]
public async Task TestPayload_10MB() { }

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…s, and clean up

Co-authored-by: CypherPotato <17441745+CypherPotato@users.noreply.github.com>
Copilot AI changed the title [WIP] Add large payload validation tests to Sisk.Core Address code review feedback for large payload tests Feb 16, 2026
Copilot AI requested a review from CypherPotato February 16, 2026 14:12
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