test: Phase 5 - Add comprehensive unit tests for Auth, Migration, and CLI E2E tests#106
Conversation
- Update project structure to include integration test projects - Update testing table to show unit vs integration test status - Add LiveTests section documenting existing coverage - Mark Auth/Migration unit tests and CLI E2E as pending 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add .env.example template for test credentials - Add Load-TestEnv.ps1 script to load .env.local into session - Update .gitignore to exclude .env.local files - Document local integration test setup in CLAUDE.md 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Update project structure to include integration test projects - Update testing table to show unit vs integration test status - Add LiveTests section documenting existing coverage - Mark Auth/Migration unit tests and CLI E2E as pending 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add .env.example template for test credentials - Add Load-TestEnv.ps1 script to load .env.local into session - Update .gitignore to exclude .env.local files - Document local integration test setup in CLAUDE.md 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add Test-NewCodeCoverage.ps1 to check new code has tests - Compares branch to main, finds new .cs files in src/ - Checks for corresponding test files in tests/ - Excludes interfaces, exceptions, options (don't need tests) - Update /pre-pr command to reference the script 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…m/joshsmithxrm/ppds-sdk into feature/testing-phase5-unit-tests
- Add Codecov badge to README - Create docs/COVERAGE_BASELINE.md with per-package baseline - Documents current coverage vs targets from Issue #55 Closes #84 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Phase 5 completion for Issue #55: PPDS.Auth.Tests (282 tests): - ProfileCollection, AuthProfile, ProfileStore, ProfileEncryption, ProfilePaths - CredentialProviderFactory, JwtClaimsParser, AuthenticationException - EnvironmentResolver, DiscoveredEnvironment - CloudEndpoints, ConnectionResolver PPDS.Migration.Tests (200 tests): - All model classes (EntitySchema, FieldSchema, etc.) - Format readers/writers (CmtSchemaReader, CmtSchemaWriter, etc.) - Analysis (DependencyGraphBuilder, ExecutionPlanBuilder) - Import/Export options and results - Progress reporters CLI E2E Tests (PPDS.LiveTests/Cli): - AuthCommandE2ETests - auth list, who, create, delete, select - EnvCommandE2ETests - env list, who, select - DataSchemaCommandE2ETests - data schema generation Updated CLAUDE.md testing table with all packages at ✅ status. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Update baseline table with actual test counts (1,147 total tests) - Mark Phase 5 checkboxes complete (Auth.Tests, Migration.Tests, CLI E2E) - Add CLI E2E coverage matrix showing current vs pending coverage - Add Future Work section with plan for data migration E2E tests - Link to Issue #104 for E2E expansion tracking - Add test project summary table 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Git history is the source of truth for document timestamps. Manual dates create maintenance burden and can become misleading. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @joshsmithxrm, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request marks the successful completion of Phase 5 of the integration testing infrastructure initiative. It significantly bolsters the project's test suite by introducing extensive unit tests for the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Pull request overview
This PR completes Phase 5 of Issue #55 by adding comprehensive test coverage across three major areas: PPDS.Auth (282 tests), PPDS.Migration (200 tests), and CLI E2E tests (27 tests). The total test count now reaches 1,147 tests across the SDK, bringing the overall estimated coverage to ~58%.
Key Changes:
- Added PPDS.Auth.Tests project covering profiles, credentials, discovery, cloud endpoints, and pooling
- Added PPDS.Migration.Tests project covering models, formats, analysis, and import/export options
- Extended PPDS.LiveTests with CLI E2E tests for auth, env, and data schema commands
- Created test infrastructure with .env.example, Load-TestEnv.ps1, and Test-NewCodeCoverage.ps1
- Updated documentation in CLAUDE.md, COVERAGE_BASELINE.md, and README.md
Reviewed changes
Copilot reviewed 50 out of 51 changed files in this pull request and generated 26 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/PPDS.Auth.Tests/PPDS.Auth.Tests.csproj | New test project targeting net8.0/9.0/10.0 with xunit 2.9.3 and FluentAssertions 8.8.0 |
| tests/PPDS.Migration.Tests/PPDS.Migration.Tests.csproj | New test project with identical dependencies and structure |
| tests/PPDS.Auth.Tests/Profiles/* | 282 tests covering ProfileCollection, AuthProfile, ProfileStore, ProfileEncryption, ProfilePaths, EnvironmentInfo |
| tests/PPDS.Migration.Tests/Models/* | 200 tests covering schema models, execution plans, dependency graphs, ID/user mappings |
| tests/PPDS.LiveTests/Cli/* | 27 E2E tests for auth, env, and data schema CLI commands |
| scripts/Load-TestEnv.ps1 | PowerShell script to load .env.local for integration tests |
| scripts/Test-NewCodeCoverage.ps1 | Pre-PR validation script checking test coverage for new code |
| docs/COVERAGE_BASELINE.md | Comprehensive coverage tracking with targets per package |
| CLAUDE.md | Updated testing requirements and local setup instructions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Code Review
This is an excellent pull request that significantly boosts the project's test coverage. The addition of comprehensive unit tests for the PPDS.Auth and PPDS.Migration packages, along with new CLI E2E tests, is a major step forward for the project's stability and maintainability. The new testing infrastructure, including the .env.example for local setup and the PowerShell scripts for loading test environments and checking coverage, is well-designed and will be very valuable for future development.
The tests themselves are thorough, well-structured, and cover a wide range of scenarios, including edge cases and error conditions. The quality of the new code is very high.
I have found one minor issue in the new code coverage script related to path handling that could make it brittle. My feedback includes a suggestion to improve its robustness. Overall, this is a fantastic contribution to the project.
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
PPDS.Cli targets net8.0;net9.0;net10.0, so dotnet run requires --framework to be specified. Use net8.0 (LTS) since CLI wrapper behavior is framework-agnostic and libraries are already tested per-TFM via LiveTests/Authentication/*, Pooling/*, etc. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add using statements for MemoryStream/StreamReader in test files (CmtSchemaReaderTests, CmtSchemaWriterTests, UserMappingReaderTests) - Remove unused variable capture in IdMappingTests.ThreadSafety_ConcurrentAdds - Add CERT to sensitive value masking pattern in Load-TestEnv.ps1 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove (Preview) from GitHub/Azure DevOps federated auth options - Remove PAC CLI references from code comments (ADR context retained) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use /comments/{id}/replies endpoint instead of base comments endpoint
with in_reply_to parameter.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
tests/PPDS.Auth.Tests/Credentials/CredentialProviderFactoryTests.cs
Dismissed
Show dismissed
Hide dismissed
CI builds with Release configuration, but dotnet run --no-build defaults to Debug. Must specify matching configuration. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Wrap client usage in try-catch to dispose on any exception, not just IsReady failure. Fixes CodeQL cs/dispose-not-called-on-throw alerts. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Fixes: - AuthList_JsonFormat: Fixed assertion to not require activeIndex (omitted from JSON when null due to WhenWritingNull serializer option) - DataSchema_InvalidEntity: CLI now fails when no entities are found instead of silently returning empty schema with exit code 0 - EnvList timeout: Removed test that times out because Global Discovery Service doesn't support service principal authentication - AuthDelete race condition: Added PPDS_CONFIG_DIR env var support and test isolation - each test instance uses unique config directory The test isolation prevents race conditions when tests run in parallel across multiple target frameworks (net8, net9, net10). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
CLI E2E tests spawn the CLI with `--framework net8.0`, so running them on net9.0 and net10.0 was testing the exact same CLI binary 3 times. This was also the source of the race condition where multiple TFMs simultaneously wrote to the shared profile store. Changes: - Add CliE2EFactAttribute: skips on non-net8.0 runtimes - Add CliE2EWithCredentialsAttribute: combines TFM + credential check - Update all CLI test classes to use new attributes - Tests now properly skip on net9/net10, run only on net8 Expected CI improvement: ~66% faster CLI E2E tests (1x instead of 3x) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
EnvList_WithActiveProfile_ListsEnvironments times out because Global Discovery Service does not support service principal authentication. The CLI hangs waiting for a response that will never come. This is the same issue as EnvList_JsonFormat_ReturnsValidJson which was already removed. Both tests require interactive auth to work. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Previously, when a service principal profile tried to use Global Discovery (via `env list` or environment resolution by name), the CLI would hang indefinitely waiting for device code authentication that would never complete. Changes: - GlobalDiscoveryService.FromProfile() now validates auth method upfront and throws NotSupportedException with clear guidance for unsupported methods - CreateTokenProviderFunction() no longer falls back to device code automatically - auth method must be explicitly InteractiveBrowser or DeviceCode - Added SupportsGlobalDiscovery() helper for consistent validation - EnvironmentResolutionService now uses the centralized validation - Added comprehensive unit tests for all auth method validation - Restored EnvList E2E test with proper error expectations The error message now clearly explains: - Why Global Discovery requires interactive auth - Which auth method the profile is using - What alternatives are available (env select --environment, auth create) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Completes Phase 5 of Issue #55 (Integration Testing Infrastructure) by adding comprehensive unit tests for PPDS.Auth and PPDS.Migration packages, plus CLI E2E tests for auth, env, and schema commands.
What's Included
New Test Projects
Test Breakdown
PPDS.Auth.Tests (282 tests):
PPDS.Migration.Tests (200 tests):
CLI E2E Tests (27 tests):
Infrastructure
.env.exampleandscripts/Load-TestEnv.ps1for local integration test setupscripts/Test-NewCodeCoverage.ps1for pre-PR validation.gitignorefor.env.localDocumentation
CLAUDE.mdtesting table (all packages now ✅)docs/COVERAGE_BASELINE.mdwith Phase 5 completionTest Results
All tests passing across net8.0, net9.0, net10.0.
Test plan
dotnet build -c Release --warnaserrorpassesdotnet test --filter "Category!=Integration"passesRelated Issues
🤖 Generated with Claude Code