Add tests #6
Merged
Add tests #6
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull request overview
This pull request significantly expands test coverage across the ProjGraph solution by adding comprehensive unit, integration, and contract tests. The changes also include improved error handling in the GraphService and removal of the legacy .sln file in favor of the .slnx format.
Changes:
- Added extensive unit tests for parsers, renderers, models, algorithms, and services
- Added integration tests for CLI commands and MCP tools with helper utilities
- Enhanced contract tests with more detailed assertions
- Improved error handling by adding FileNotFoundException check in GraphService
- Removed ProjGraph.sln file (likely moving to .slnx exclusively)
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/ProjGraph.Tests.Unit/Services/GraphServiceTests.cs | Added tests for error cases and edge conditions; removed platform-specific skip logic |
| tests/ProjGraph.Tests.Unit/Services/EfAnalysisServiceTests.cs | Added comprehensive tests for Entity Framework analysis edge cases |
| tests/ProjGraph.Tests.Unit/Rendering/MermaidGraphRendererTests.cs | New file with complete test coverage for Mermaid graph rendering |
| tests/ProjGraph.Tests.Unit/Rendering/MermaidErdRendererTests.cs | New file with complete test coverage for Mermaid ERD rendering |
| tests/ProjGraph.Tests.Unit/Parsers/SlnxParserTests.cs | Added tests for edge cases and cross-platform path handling |
| tests/ProjGraph.Tests.Unit/Parsers/SlnParserTests.cs | Minor code style improvement (const instead of var) |
| tests/ProjGraph.Tests.Unit/Parsers/ProjectParserTests.cs | Added tests for project parsing edge cases; unified path separators to forward slashes |
| tests/ProjGraph.Tests.Unit/Models/SolutionGraphTests.cs | New file with tests for SolutionGraph model behavior |
| tests/ProjGraph.Tests.Unit/Models/ProjectTests.cs | New file with tests for Project model behavior |
| tests/ProjGraph.Tests.Unit/Models/DependencyTests.cs | New file with tests for Dependency model behavior |
| tests/ProjGraph.Tests.Unit/Algorithms/TarjanSccAlgorithmTests.cs | Added tests for algorithm edge cases including self-loops and complex graphs |
| tests/ProjGraph.Tests.Unit/ProjGraph.Tests.Unit.csproj | Formatting change removing spaces before self-closing tags |
| tests/ProjGraph.Tests.Integration/Mcp/McpIntegrationTests.cs | Refactored tests with helper methods and added comprehensive test scenarios |
| tests/ProjGraph.Tests.Integration/Mcp/McpErdTests.cs | Added extensive ERD integration tests including error handling |
| tests/ProjGraph.Tests.Integration/Cli/VisualizeCommandTests.cs | New file with CLI visualization command tests |
| tests/ProjGraph.Tests.Integration/Cli/ErdCommandTests.cs | New file with CLI ERD command tests |
| tests/ProjGraph.Tests.Integration/Cli/CliTestHelpers.cs | New helper utilities for CLI testing |
| tests/ProjGraph.Tests.Integration/Cli/CliIntegrationTests.cs | Deleted file (functionality moved to separate command test files) |
| tests/ProjGraph.Tests.Contract/McpErdContractTests.cs | Enhanced with more detailed attribute and parameter validation |
| tests/ProjGraph.Tests.Contract/McpContractTests.cs | Enhanced with more detailed attribute and parameter validation |
| src/ProjGraph.Lib/Services/GraphService.cs | Added FileNotFoundException check for improved error handling |
| ProjGraph.sln | Deleted legacy solution file |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| // Arrange | ||
| var tools = CreateTools(); | ||
| const string nonExistentPath = @"C:\this\path\does\not\exist.cs"; |
There was a problem hiding this comment.
Hard-coded Windows path in non-Windows specific test. The path "C:\this\path\does\not\exist.cs" will fail on non-Windows systems. Use Path.Combine or Path.GetTempPath() with subdirectories instead to ensure cross-platform compatibility.
Suggested change
| const string nonExistentPath = @"C:\this\path\does\not\exist.cs"; | |
| var nonExistentPath = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N") + ".cs"); | |
| if (File.Exists(nonExistentPath)) | |
| { | |
| File.Delete(nonExistentPath); | |
| } |
| var output = new StringBuilder(); | ||
| var originalOut = Console.Out; | ||
| var originalError = Console.Error; | ||
| var writer = new StringWriter(output); |
There was a problem hiding this comment.
This variable is manually disposed in a finally block - consider a C# using statement as a preferable resource management technique.
…mprove path handling in GraphService
…platform compatibility
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.