Refactor dashboard automation tests and improve configuration handling #4238
Conversation
There was a problem hiding this comment.
Pull Request Overview
Refactors the .NET Aspire dashboard automation tests to improve configuration handling and fix various issues with test stability and reliability.
- Improved configuration value retrieval with default fallback values
- Enhanced URL resolution using proper event-driven mechanisms
- Increased test timeout and improved error handling for better reliability
Reviewed Changes
Copilot reviewed 8 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
Program.cs |
Updated configuration value retrieval to use overload with default value |
PlaywrightTestsBase.cs |
Refactored URL resolution using event-driven approach and added error handling |
PlaywrightFixture.cs |
Increased default timeout from 10 to 30 seconds |
FluentDataGridSelector.cs |
Added debugger display attribute for better debugging experience |
PageExtensions.cs |
Enhanced login method with network idle wait condition |
DashboardSelectors.cs |
Updated selector to handle both C: and E: drive paths |
CaptureImages.cs |
Simplified test code and improved reliability with forced clicks |
AppHostTestFixture.cs |
Improved configuration handling and added unsecured transport option |
| if (addresses is not null) | ||
| { | ||
|
|
||
| } |
There was a problem hiding this comment.
This code block from lines 40-45 appears to be unused dead code. The server variable and addresses are retrieved but never used, and the block is empty. This should be removed to improve code clarity.
| } |
Copilot uses AI. Check for mistakes.
| @@ -1,5 +1,7 @@ | |||
| namespace Aspire.Dashboard.ScreenCapture; | |||
| | |||
There was a problem hiding this comment.
[nitpick] The empty line at the beginning of the file should be removed to maintain consistent file formatting.
| |
Copilot uses AI. Check for mistakes.
|
|
||
| [DebuggerDisplay($"{{{nameof(GetDebuggerDisplay)}(),nq}}")] |
There was a problem hiding this comment.
The DebuggerDisplay attribute is missing the required using statement for System.Diagnostics. This will cause a compilation error.
Copilot uses AI. Check for mistakes.
| DashboardSelectors.SplitPanels, DashboardSelectors.MedianId, (0, 20)); | ||
| //await page.AdjustSplitPanelsGridTemplateAsync(); | ||
| //await page.ClickAndDragShadowRootElementAsync( | ||
| // DashboardSelectors.SplitPanels, DashboardSelectors.MedianId, (0, 20)); |
There was a problem hiding this comment.
Multiple lines of commented-out code (lines 197, 199-201) should be removed rather than left as comments. If this code might be needed later, consider using version control history instead.
Copilot uses AI. Check for mistakes.
Summary
Refactor dashboard automation tests and improve configuration handling
Fixes #4237