Skip to content

Conversation

@carldebilly
Copy link
Owner

  • Introduced an end-to-end testing setup using a Node.js script.
  • Created an E2E test application (E2EApp) to validate HTTP server responses (/ping, /sse/js).
  • Added integration for E2E tests within the test suite (E2E_NodeFixture).
  • Updated GitHub Actions to collect code coverage during tests.
  • Registered new handlers for E2E scenarios and improved modularity (SSE, ping).

- Introduced an end-to-end testing setup using a Node.js script.
- Created an E2E test application (`E2EApp`) to validate HTTP server responses (`/ping`, `/sse/js`).
- Added integration for E2E tests within the test suite (`E2E_NodeFixture`).
- Updated GitHub Actions to collect code coverage during tests.
- Registered new handlers for E2E scenarios and improved modularity (`SSE`, `ping`).
@carldebilly carldebilly requested a review from Copilot September 6, 2025 06:02
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a comprehensive end-to-end testing framework using Node.js to validate the HTTP server functionality from a consumer perspective. The implementation includes a dedicated E2E test application, integration with the existing test suite, and enhanced CI configuration.

  • Adds E2E testing infrastructure with Node.js script to test real HTTP server scenarios
  • Creates dedicated E2EApp console application to serve as test target
  • Integrates E2E tests into existing MSTest framework with proper Node.js dependency checking

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
Yllibed.HttpServer.Tests/e2e/run-e2e.js Node.js E2E test runner that validates /ping and SSE endpoints
Yllibed.HttpServer.Tests/E2E_NodeFixture.cs MSTest fixture that executes the Node.js E2E script
Yllibed.HttpServer.Tests/E2EApp/Program.cs Test application serving /ping and /sse/js endpoints
Yllibed.HttpServer.Tests/E2EApp/E2EApp.csproj Project configuration for the E2E test application
Yllibed.HttpServer.Tests/Yllibed.HttpServer.Tests.csproj Updated test project to include E2E files
Yllibed.HttpServer.Json.Tests/JsonHandlerBaseMoreFixture.cs Additional JSON handler test scenarios
.github/workflows/build.yml Added code coverage collection to CI workflow

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

protected override async Task HandleSseSession(ISseSession sse, CancellationToken ct)
{
// Emit a simple event with a fixed id/name and JSON payload
// Emit compact JSON manually to avoid extra project references
Copy link

Copilot AI Sep 6, 2025

Choose a reason for hiding this comment

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

[nitpick] The comment formatting is inconsistent with the indentation pattern used in the rest of the file. This comment should be aligned with the same indentation as the code it describes.

Suggested change
// Emit compact JSON manually to avoid extra project references
// Emit compact JSON manually to avoid extra project references

Copilot uses AI. Check for mistakes.
process.exitCode = 1;
} finally {
if (process.platform === 'win32') {
spawn('taskkill', ['/PID', String(app.pid), '/T', '/F']);
Copy link

Copilot AI Sep 6, 2025

Choose a reason for hiding this comment

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

The spawned taskkill process is not awaited or handled for errors. If taskkill fails, the child process may remain running. Consider using synchronous execution or proper error handling.

Suggested change
spawn('taskkill', ['/PID', String(app.pid), '/T', '/F']);
const { spawnSync } = require('child_process');
const result = spawnSync('taskkill', ['/PID', String(app.pid), '/T', '/F']);
if (result.error || result.status !== 0) {
console.error('Failed to kill process with taskkill:', result.error || result.stderr.toString());
}

Copilot uses AI. Check for mistakes.
// Heuristic: tests usually run with a WorkingDirectory under the test project's bin folder.
// Walk up until we find the solution file.
var dir = new DirectoryInfo(Environment.CurrentDirectory);
for (int i = 0; i < 10 && dir is not null; i++, dir = dir.Parent!)
Copy link

Copilot AI Sep 6, 2025

Choose a reason for hiding this comment

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

[nitpick] The magic number 10 for directory traversal limit should be extracted as a named constant to make the code more maintainable and self-documenting.

Copilot uses AI. Check for mistakes.
@carldebilly carldebilly marked this pull request as draft September 6, 2025 06:05
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