Skip to content

Commit 7c77fd3

Browse files
joshsmithxrmclaude
andauthored
feat: add integration testing infrastructure and coverage reporting (#85)
* feat: add integration testing infrastructure (Phase 1 of #55) Add three new test projects for comprehensive integration testing: - PPDS.Dataverse.IntegrationTests: FakeXrmEasy v3.8.0 for mocked Dataverse tests - PPDS.Auth.IntegrationTests: Authentication flow validation - PPDS.LiveTests: Live Dataverse connection tests with skip logic Infrastructure includes: - FakeXrmEasyTestsBase with middleware builder (RPL-1.5 license) - LiveTestBase with [Trait("Category", "Integration")] for filtering - SkipIfNoCredentials attributes for graceful credential handling - LiveTestConfiguration reading from environment variables CI/CD changes: - integration-tests.yml: Approach B workflow (trusted PRs get integration tests) - test.yml: Updated to exclude integration tests with --filter - dependabot.yml: Added fakexrmeasy group for package updates 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat: add code coverage reporting with Codecov (#84) Add coverage collection and Codecov integration for visibility into test coverage across all packages. Changes: - test.yml: Add --collect:"XPlat Code Coverage" and Codecov upload - codecov.yml: Per-package thresholds matching issue #55 targets - .gitignore: Exclude coverage/ directory Per-package targets (informational only, no PR blocking): - PPDS.Plugins: 95% (currently 100%) - PPDS.Auth: 70% (currently 0% - no tests) - PPDS.Dataverse: 60% (currently 38%) - PPDS.Migration: 50% (currently 0% - no tests) - PPDS.Cli: 60% (currently ~12%) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: pre-commit hook error on Windows - Change matcher from "Bash" to "Bash(git commit:*)" so hook only fires on commit commands, not every Bash invocation - Use relative path instead of $CLAUDE_PROJECT_DIR which wasn't expanding correctly on Windows - Simplify Python script since matcher now handles filtering 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: address bot review findings - FakeXrmEasyTestsBase: Call Context.Dispose() to prevent resource leak - LiveDataverseSmokeTests: Fix tautology, improve assertion logic - LiveDataverseSmokeTests: Remove redundant [Trait] (inherited from base) - AuthenticationSmokeTests: Remove Integration trait (no external deps) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs: add individual comment reply syntax to review-bot-comments Add section 5 documenting the correct gh api syntax for replying to individual PR review comments using in_reply_to parameter. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: IXrmFakedContext doesn't implement IDisposable directly Gemini's suggestion was incorrect - IXrmFakedContext interface doesn't declare IDisposable. Use safe cast to dispose if runtime type supports it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs: require user approval before implementing bot suggestions Update review-bot-comments command to make the process explicit: 1. Fetch and triage comments 2. Present summary with recommendations 3. WAIT for user approval (do NOT auto-implement) 4. Implement approved changes 5. Build and verify before committing 6. Reply to comments Added warning about bot suggestions being wrong (e.g., Gemini suggested calling Dispose on an interface that doesn't implement IDisposable). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: missing closing paren in settings.local.example.json 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * chore: clean up scripts folder and add certificate generator - Rename install-local.ps1 to Install-LocalCli.ps1 (PowerShell Verb-Noun convention) - Delete ppds-dev.ps1 (trivial wrapper that just called dotnet run) - Add New-TestCertificate.ps1 for generating Azure App Registration test certificates 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 26840ad commit 7c77fd3

23 files changed

+1026
-75
lines changed

.claude/commands/review-bot-comments.md

Lines changed: 57 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -15,47 +15,80 @@ gh api repos/joshsmithxrm/ppds-sdk/pulls/[PR]/comments
1515

1616
### 2. Triage Each Comment
1717

18-
For each bot comment, determine:
18+
For each bot comment, determine verdict and rationale:
1919

20-
| Verdict | Action |
21-
|---------|--------|
22-
| **Valid** | Fix the issue, reply "Fixed in [commit]" |
23-
| **False Positive** | Reply with reason, dismiss |
24-
| **Unclear** | Investigate before deciding |
20+
| Verdict | Meaning |
21+
|---------|---------|
22+
| **Valid** | Bot is correct, code should be changed |
23+
| **False Positive** | Bot is wrong, explain why |
24+
| **Unclear** | Need to investigate before deciding |
2525

26-
### 3. Common False Positives
26+
### 3. Present Summary and WAIT FOR APPROVAL
27+
28+
**CRITICAL: Do NOT implement fixes automatically.**
29+
30+
Present a summary table to the user:
31+
32+
```markdown
33+
## Bot Review Triage - PR #XX
34+
35+
| # | Bot | Finding | Verdict | Recommendation | Rationale |
36+
|---|-----|---------|---------|----------------|-----------|
37+
| 1 | Gemini | Missing Dispose | Valid | Add dispose call | Prevents resource leak |
38+
| 2 | Copilot | Use .Where() | False Positive | Decline | Style preference |
39+
```
40+
41+
**STOP HERE. Wait for user to review and approve before making ANY changes.**
42+
43+
Bot suggestions can be wrong (e.g., suggesting methods that don't exist on an interface).
44+
Always get user approval, then verify changes compile before committing.
45+
46+
### 4. Implement Approved Changes
47+
48+
After user approval:
49+
1. Make the approved code changes
50+
2. **Build and verify** - `dotnet build` must succeed
51+
3. Run tests to confirm no regressions
52+
4. Commit with descriptive message
53+
54+
### 5. Reply to Each Comment Individually
55+
56+
After changes are committed, reply to each bot comment. Do NOT batch responses into a single PR comment.
57+
58+
```bash
59+
# Reply to a specific review comment
60+
gh api repos/joshsmithxrm/ppds-sdk/pulls/{pr}/comments \
61+
-f body="Fixed in abc123" \
62+
-F in_reply_to={comment_id}
63+
```
64+
65+
**Important:** The `in_reply_to` parameter must be the comment ID (numeric), not a URL. Get IDs from the fetch step.
66+
67+
| Verdict | Reply Template |
68+
|---------|----------------|
69+
| Valid (fixed) | `Fixed in {commit_sha} - {brief description}` |
70+
| Declined | `Declining - {reason}` |
71+
| False positive | `False positive - {explanation}` |
72+
73+
## Common False Positives
2774

2875
| Bot Claim | Why It's Often Wrong |
2976
|-----------|---------------------|
3077
| "Use .Where() instead of foreach+if" | Preference, not correctness |
3178
| "Volatile needed with Interlocked" | Interlocked provides barriers |
3279
| "OR should be AND" | Logic may be intentionally inverted (DeMorgan) |
3380
| "Static field not thread-safe" | May be set once at startup |
81+
| "Call Dispose on X" | Interface may not actually implement IDisposable |
3482

35-
### 4. Common Valid Findings
83+
## Common Valid Findings
3684

3785
| Pattern | Usually Valid |
3886
|---------|---------------|
3987
| Unused variable/parameter | Yes - dead code |
4088
| Missing null check | Check context |
41-
| Resource not disposed | Yes - leak |
89+
| Resource not disposed | Yes - but verify interface first |
4290
| Generic catch clause | Context-dependent |
4391

44-
## Output
45-
46-
```markdown
47-
## Bot Review Triage - PR #82
48-
49-
| # | Bot | Finding | Verdict | Action |
50-
|---|-----|---------|---------|--------|
51-
| 1 | Gemini | Use constants in dict | Valid | Fixed in abc123 |
52-
| 2 | Copilot | Add validation tests | Valid | Fixed in def456 |
53-
| 3 | Copilot | Use .Where() | False Positive | Style preference |
54-
| 4 | CodeQL | Generic catch | Valid (low) | Acceptable for disposal |
55-
56-
All findings addressed: Yes
57-
```
58-
5992
## When to Use
6093

6194
- After opening a PR (before requesting review)

.claude/hooks/pre-commit-validate.py

Lines changed: 21 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,47 +2,36 @@
22
"""
33
Pre-commit validation hook for PPDS SDK.
44
Runs dotnet build and test before allowing git commit.
5+
6+
Note: This hook is only triggered for 'git commit' commands via the
7+
matcher in .claude/settings.json - no need to filter here.
58
"""
6-
import json
7-
import shlex
89
import subprocess
910
import sys
1011
import os
12+
import json
1113

12-
def main():
13-
try:
14-
input_data = json.load(sys.stdin)
15-
except json.JSONDecodeError:
16-
print("⚠️ pre-commit-validate: Failed to parse input. Skipping validation.", file=sys.stderr)
17-
sys.exit(0)
18-
19-
tool_name = input_data.get("tool_name", "")
20-
tool_input = input_data.get("tool_input", {})
21-
command = tool_input.get("command", "")
2214

23-
# Only validate git commit commands (robust check using shlex)
24-
if tool_name != "Bash":
25-
sys.exit(0)
15+
def main():
16+
# Read stdin (Claude Code sends JSON with tool info)
2617
try:
27-
parts = shlex.split(command)
28-
is_git_commit = len(parts) >= 2 and os.path.basename(parts[0]) == "git" and parts[1] == "commit"
29-
except ValueError:
30-
is_git_commit = False
31-
if not is_git_commit:
32-
sys.exit(0) # Allow non-commit commands
18+
json.load(sys.stdin)
19+
except (json.JSONDecodeError, EOFError):
20+
pass # Input parsing is optional - matcher already filtered
3321

34-
# Get project directory
22+
# Get project directory from environment or use current directory
3523
project_dir = os.environ.get("CLAUDE_PROJECT_DIR", os.getcwd())
3624

3725
try:
26+
print("🔨 Running pre-commit validation...", file=sys.stderr)
27+
3828
# Run dotnet build
39-
print("Running pre-commit validation...", file=sys.stderr)
4029
build_result = subprocess.run(
4130
["dotnet", "build", "-c", "Release", "--nologo", "-v", "q"],
4231
cwd=project_dir,
4332
capture_output=True,
4433
text=True,
45-
timeout=300 # 5 minute timeout
34+
timeout=300
4635
)
4736

4837
if build_result.returncode != 0:
@@ -51,16 +40,16 @@ def main():
5140
print(build_result.stdout, file=sys.stderr)
5241
if build_result.stderr:
5342
print(build_result.stderr, file=sys.stderr)
54-
sys.exit(2) # Block commit
43+
sys.exit(2)
5544

56-
# Run dotnet test (unit tests only - integration tests run on PR)
45+
# Run unit tests only (integration tests run on PR)
5746
test_result = subprocess.run(
5847
["dotnet", "test", "--no-build", "-c", "Release", "--nologo", "-v", "q",
5948
"--filter", "Category!=Integration"],
6049
cwd=project_dir,
6150
capture_output=True,
6251
text=True,
63-
timeout=300 # 5 minute timeout
52+
timeout=300
6453
)
6554

6655
if test_result.returncode != 0:
@@ -69,17 +58,18 @@ def main():
6958
print(test_result.stdout, file=sys.stderr)
7059
if test_result.stderr:
7160
print(test_result.stderr, file=sys.stderr)
72-
sys.exit(2) # Block commit
61+
sys.exit(2)
7362

7463
print("✅ Build and unit tests passed", file=sys.stderr)
75-
sys.exit(0) # Allow commit
64+
sys.exit(0)
7665

7766
except FileNotFoundError:
78-
print("⚠️ pre-commit-validate: dotnet not found in PATH. Skipping validation.", file=sys.stderr)
67+
print("⚠️ dotnet not found in PATH. Skipping validation.", file=sys.stderr)
7968
sys.exit(0)
8069
except subprocess.TimeoutExpired:
81-
print("⚠️ pre-commit-validate: Build/test timed out after 5 minutes. Skipping validation.", file=sys.stderr)
70+
print("⚠️ Build/test timed out. Skipping validation.", file=sys.stderr)
8271
sys.exit(0)
8372

73+
8474
if __name__ == "__main__":
8575
main()

.claude/settings.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@
22
"hooks": {
33
"PreToolUse": [
44
{
5-
"matcher": "Bash",
5+
"matcher": "Bash(git commit:*)",
66
"hooks": [
77
{
88
"type": "command",
9-
"command": "python \"$CLAUDE_PROJECT_DIR/.claude/hooks/pre-commit-validate.py\"",
9+
"command": "python .claude/hooks/pre-commit-validate.py",
1010
"timeout": 120
1111
}
1212
]

.claude/settings.local.example.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@
7979
"Bash(gh pr status:*)",
8080
"Bash(gh pr checkout:*)",
8181
"Bash(gh pr diff:*)",
82-
"Bash(gh api:*",
82+
"Bash(gh api:*)",
8383
"Bash(gh issue list:*)",
8484
"Bash(gh issue view:*)",
8585
"Bash(gh issue status:*)",

.github/dependabot.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ updates:
2727
dataverse:
2828
patterns:
2929
- "Microsoft.PowerPlatform.Dataverse.*"
30+
fakexrmeasy:
31+
patterns:
32+
- "FakeXrmEasy*"
3033
# Widen version constraints when needed (e.g., 1.1.* -> 1.2.*)
3134
versioning-strategy: increase
3235

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
name: Integration Tests
2+
3+
on:
4+
push:
5+
branches: [main]
6+
pull_request:
7+
branches: [main]
8+
9+
jobs:
10+
unit-tests:
11+
name: Unit Tests
12+
runs-on: windows-latest
13+
14+
steps:
15+
- uses: actions/checkout@v6
16+
with:
17+
fetch-depth: 0
18+
19+
- name: Setup .NET
20+
uses: actions/setup-dotnet@v5
21+
with:
22+
dotnet-version: |
23+
8.0.x
24+
9.0.x
25+
10.0.x
26+
27+
- name: Restore dependencies
28+
run: dotnet restore
29+
30+
- name: Build
31+
run: dotnet build --configuration Release --no-restore
32+
33+
- name: Run Unit Tests (exclude integration)
34+
run: dotnet test --configuration Release --no-build --verbosity normal --filter "Category!=Integration"
35+
36+
integration-tests:
37+
name: Integration Tests
38+
runs-on: windows-latest
39+
# Only run on PRs from the same repo (not forks) or on main branch
40+
if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository
41+
environment: test-dataverse
42+
needs: unit-tests
43+
44+
steps:
45+
- uses: actions/checkout@v6
46+
with:
47+
fetch-depth: 0
48+
49+
- name: Setup .NET
50+
uses: actions/setup-dotnet@v5
51+
with:
52+
dotnet-version: |
53+
8.0.x
54+
9.0.x
55+
10.0.x
56+
57+
- name: Restore dependencies
58+
run: dotnet restore
59+
60+
- name: Build
61+
run: dotnet build --configuration Release --no-restore
62+
63+
- name: Run Integration Tests
64+
env:
65+
DATAVERSE_URL: ${{ secrets.DATAVERSE_URL }}
66+
PPDS_TEST_APP_ID: ${{ secrets.PPDS_TEST_APP_ID }}
67+
PPDS_TEST_CLIENT_SECRET: ${{ secrets.PPDS_TEST_CLIENT_SECRET }}
68+
PPDS_TEST_TENANT_ID: ${{ secrets.PPDS_TEST_TENANT_ID }}
69+
PPDS_TEST_CERT_BASE64: ${{ secrets.PPDS_TEST_CERT_BASE64 }}
70+
PPDS_TEST_CERT_PASSWORD: ${{ secrets.PPDS_TEST_CERT_PASSWORD }}
71+
run: dotnet test --configuration Release --no-build --verbosity normal --filter "Category=Integration"

.github/workflows/test.yml

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,5 +29,14 @@ jobs:
2929
- name: Build
3030
run: dotnet build --configuration Release --no-restore
3131

32-
- name: Test
33-
run: dotnet test --configuration Release --no-build --verbosity normal
32+
- name: Test with Coverage
33+
run: |
34+
dotnet test --configuration Release --no-build --verbosity normal --filter "Category!=Integration" --collect:"XPlat Code Coverage" --results-directory ./coverage
35+
36+
- name: Upload coverage to Codecov
37+
uses: codecov/codecov-action@v5
38+
with:
39+
directory: ./coverage
40+
flags: unittests
41+
fail_ci_if_error: false # Don't fail build if upload fails
42+
verbose: true

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,3 +55,6 @@ nul
5555
data.zip
5656
data/
5757
schema.xml
58+
59+
# Code coverage
60+
coverage/

0 commit comments

Comments
 (0)