feat: implement execution comparison feature with UI and API integration#14
Conversation
f2fc492 to
dbb3b9a
Compare
There was a problem hiding this comment.
Pull request overview
This PR implements a comprehensive execution comparison feature that enables users to compare performance metrics between two load test executions. The feature includes a new API endpoint, service layer logic for calculating performance deltas, and a complete UI with a modal interface and visualization charts.
Changes:
- New comparison API endpoint that accepts baseline and comparison execution IDs and returns detailed performance delta calculations
- Client-side comparison modal with execution selection UI and comprehensive results display including metrics table and percentile charts
- CSS styling for the comparison modal with responsive design for mobile devices
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
Commands/Server/ServerCommand.cs |
Adds POST endpoint /api/executions/compare for comparing executions |
Commands/Server/ServerSettings.cs |
Changes default port from 5000 to 5002 |
Web/Services/ExecutionService.cs |
Implements CompareExecutionsAsync method with delta calculation and assessment logic |
Web/Models.cs |
Adds data models: ExecutionComparisonRequest, ExecutionComparisonResult, ExecutionSummary, and PerformanceDelta |
Web/JsonContext.cs |
Registers new comparison-related types for JSON serialization |
wwwroot/js/app.js |
Implements comparison modal functionality, execution selection, result rendering, and chart visualization |
wwwroot/index.html |
Adds comparison modal HTML structure with selection interface and results grid |
wwwroot/css/styles.css |
Adds comprehensive styling for comparison modal with responsive design |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private static string DetermineAssessment(double rpsDelta, double avgResponseDelta, double baselineRps, double baselineAvgResponse) | ||
| { | ||
| var rpsPercentChange = baselineRps > 0 ? (rpsDelta / baselineRps) * 100 : 0; | ||
| var responsePercentChange = baselineAvgResponse > 0 ? (avgResponseDelta / baselineAvgResponse) * 100 : 0; | ||
|
|
||
| var rpsImproved = rpsPercentChange > 5; | ||
| var rpsRegressed = rpsPercentChange < -5; | ||
| var responseImproved = responsePercentChange < -5; | ||
| var responseRegressed = responsePercentChange > 5; | ||
|
|
||
| if (rpsImproved && responseImproved) | ||
| return "Significant Improvement"; | ||
| if (rpsImproved || responseImproved) | ||
| return "Improvement"; | ||
| if (rpsRegressed && responseRegressed) | ||
| return "Significant Regression"; | ||
| if (rpsRegressed || responseRegressed) | ||
| return "Regression"; | ||
| return "No Significant Change"; | ||
| } |
There was a problem hiding this comment.
The DetermineAssessment method uses hardcoded threshold values of 5% without explanation or configuration. These thresholds may be too high or too low depending on the use case. Consider making these thresholds configurable, or at least document why 5% was chosen as the threshold for significant change. Additionally, the logic doesn't account for the failure rate delta, which could be a critical indicator of regression.
| overlay.classList.add('visible'); | ||
|
|
||
| try { | ||
| const response = await fetch(`/api/endpoints/${endpointId}/executions?page=1&pageSize=50`); |
There was a problem hiding this comment.
The fetch request for executions doesn't check if the response is ok before parsing JSON. If the API returns a 404 or 500 error, the code will fail when trying to parse the error response as JSON. Add a response.ok check similar to the one added in the showExecutionDetails method at line 785.
| const response = await fetch(`/api/endpoints/${endpointId}/executions?page=1&pageSize=50`); | |
| const response = await fetch(`/api/endpoints/${endpointId}/executions?page=1&pageSize=50`); | |
| if (!response.ok) { | |
| throw new Error(`Failed to load executions: ${response.status} ${response.statusText}`); | |
| } |
| <label>Baseline Execution</label> | ||
| <select id="baselineExecution" onchange="window.app.updateCompareButton()"> | ||
| <option value="">Select baseline...</option> | ||
| </select> | ||
| </div> | ||
| <div class="comparison-vs">VS</div> | ||
| <div class="comparison-selector"> | ||
| <label>Compare Execution</label> |
There was a problem hiding this comment.
The label elements are missing the 'for' attribute to associate them with their respective select elements. This impacts accessibility for screen reader users. Add for="baselineExecution" to line 739 and for="compareExecution" to line 746.
| <label>Baseline Execution</label> | |
| <select id="baselineExecution" onchange="window.app.updateCompareButton()"> | |
| <option value="">Select baseline...</option> | |
| </select> | |
| </div> | |
| <div class="comparison-vs">VS</div> | |
| <div class="comparison-selector"> | |
| <label>Compare Execution</label> | |
| <label for="baselineExecution">Baseline Execution</label> | |
| <select id="baselineExecution" onchange="window.app.updateCompareButton()"> | |
| <option value="">Select baseline...</option> | |
| </select> | |
| </div> | |
| <div class="comparison-vs">VS</div> | |
| <div class="comparison-selector"> | |
| <label for="compareExecution">Compare Execution</label> |
| [DefaultValue(5002)] | ||
| [Description("Define the port for the web server. Default is 5002.")] |
There was a problem hiding this comment.
The default port has been changed from 5000 to 5002 without explanation in the PR description. This appears to be an unintended change that is unrelated to the execution comparison feature. Changing the default port is a breaking change for existing users who may have configurations, scripts, or firewall rules based on port 5000. If this change is intentional, it should be documented in the PR description with justification.
| [DefaultValue(5002)] | |
| [Description("Define the port for the web server. Default is 5002.")] | |
| [DefaultValue(5000)] | |
| [Description("Define the port for the web server. Default is 5000.")] |
| getAssessmentIcon(assessment) { | ||
| if (assessment.includes('Improvement')) return 'chart-line'; | ||
| if (assessment.includes('Regression')) return 'chart-line'; | ||
| return 'equals'; | ||
| } | ||
|
|
There was a problem hiding this comment.
The getAssessmentIcon function is defined but never called anywhere in the codebase. This is dead code that should be removed to improve maintainability, or if it's intended for future use, it should be implemented now as part of this feature.
| getAssessmentIcon(assessment) { | |
| if (assessment.includes('Improvement')) return 'chart-line'; | |
| if (assessment.includes('Regression')) return 'chart-line'; | |
| return 'equals'; | |
| } |
| }); | ||
|
|
||
| app.MapPost("/api/executions/compare", async (ExecutionComparisonRequest request, ExecutionService service) => | ||
| { |
There was a problem hiding this comment.
The comparison endpoint lacks input validation. The API should validate that baselineExecutionId and compareExecutionId are positive integers and that they are not equal to each other. While the UI prevents selecting the same execution twice, API endpoints should always validate inputs independently of the client.
| { | |
| { | |
| if (request is null) | |
| { | |
| return Results.BadRequest(new ErrorResponse("Request body is required")); | |
| } | |
| if (request.BaselineExecutionId <= 0 || request.CompareExecutionId <= 0) | |
| { | |
| return Results.BadRequest(new ErrorResponse("BaselineExecutionId and CompareExecutionId must be positive integers")); | |
| } | |
| if (request.BaselineExecutionId == request.CompareExecutionId) | |
| { | |
| return Results.BadRequest(new ErrorResponse("BaselineExecutionId and CompareExecutionId must not be equal")); | |
| } |
feat: implement execution comparison feature with UI and API integration
Description
This PR implements a complete load test execution comparison feature, allowing users to analyze performance differences between two executions of the same endpoint.
Features Added
POST /api/executions/compareto compare two executionsFiles Modified
Commands/Server/ServerCommand.csCommands/Server/ServerSettings.csWeb/JsonContext.csWeb/Models.csExecutionComparisonRequest,ExecutionComparisonResult,ExecutionSummaryandPerformanceDeltamodelsWeb/Services/ExecutionService.csCompareExecutionsAsync()with delta calculation logicwwwroot/css/styles.csswwwroot/index.htmlwwwroot/js/app.jsHow to Test
dotnet run -- serverChecklist
.editorconfig)