attempt to fix release pipeline#327
Conversation
WalkthroughThe solution and CI were updated for a test project rename and relocation: Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/action.yml(2 hunks).github/workflows/release.yml(1 hunks)NuGetUtility.sln(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: check_licenses (net10.0, App)
- GitHub Check: check_licenses (net8.0, App)
- GitHub Check: check_licenses (net10.0, ProjectWithReferenceContainingFileLicense)
- GitHub Check: check_licenses (net9.0, ProjectWithReferenceContainingLicenseExpression)
- GitHub Check: check_licenses (net8.0, Tests)
- GitHub Check: check_licenses (net9.0, ProjectWithReferenceContainingFileLicense)
- GitHub Check: check_licenses_net472 (App)
- GitHub Check: check_licenses_net472 (ProjectWithReferenceContainingFileLicense)
- GitHub Check: check_licenses_net472 (Tests)
- GitHub Check: check_version_command_net472
- GitHub Check: check_licenses_net472 (ProjectWithReferenceContainingLicenseExpression)
- GitHub Check: test_windows (net10.0)
- GitHub Check: test_windows (net9.0)
- GitHub Check: test_windows (net472)
- GitHub Check: test (macos-latest, net9.0)
- GitHub Check: test_windows (net8.0)
- GitHub Check: test (macos-latest, net10.0)
- GitHub Check: test_file_license_matching (ubuntu-latest)
- GitHub Check: test_file_license_matching (windows-latest)
- GitHub Check: test_file_license_matching (macos-latest)
🔇 Additional comments (3)
.github/workflows/action.yml (2)
92-92: LGTM!The test project path correctly reflects the renamed project from
NuGetUtility.Test.UrlToLicenseMappingtoNuGetUtility.UrlToLicenseMapping.Test.
119-119: LGTM!The explicit assembly list appropriately excludes
NuGetUtility.UrlToLicenseMapping.Test.dll, which requires Chrome and is tested separately in thetest_url_to_license_mappingjob (lines 74-92). This makes the Windows test job more reliable by avoiding environment-specific test dependencies.NuGetUtility.sln (1)
50-50: LGTM!The project rename from
NuGetUtility.Test.UrlToLicenseMappingtoNuGetUtility.UrlToLicenseMapping.Testfollows standard .NET naming conventions (Project.Test format) and correctly preserves the project GUID{A77F9673-8296-4680-A713-8AC26B7DE41C}, ensuring existing references and configurations remain intact.
| uses: josepho0918/vstest-action@main | ||
| with: | ||
| testAssembly: "*.Test.*dll" | ||
| testAssembly: "NuGet*.Test.dll" |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that test coverage is complete with the new pattern.
The pattern has been narrowed from *.Test.*dll to NuGet*.Test.dll. This will match assemblies like NuGetUtility.Test.dll, NuGetLicense.Test.dll, and NuGetUtility.UrlToLicenseMapping.Test.dll, but will exclude SPDXLicenseMatcher.Test.dll if it exists.
Run the following script to verify which test assemblies exist and confirm the pattern matches all intended test projects:
🏁 Script executed:
#!/bin/bash
# Find all test DLLs that would be in the Release build output
echo "=== Test assemblies that exist in solution ==="
fd -e csproj -t f 'Test.csproj$' tests/
echo ""
echo "=== Assemblies matching new pattern NuGet*.Test.dll ==="
rg -g 'NuGetUtility.sln' --no-filename 'Project.*Test.*csproj' | gawk 'match($0, /"([^"]+\.Test)[^"]*\.csproj"/, arr) {print arr[1] ".dll"}' | grep '^NuGet.*\.Test\.dll$'
echo ""
echo "=== Assemblies potentially excluded by new pattern ==="
rg -g 'NuGetUtility.sln' --no-filename 'Project.*Test.*csproj' | gawk 'match($0, /"([^"]+\.Test)[^"]*\.csproj"/, arr) {print arr[1] ".dll"}' | grep -v '^NuGet.*\.Test\.dll$'Length of output: 700
I need to verify whether SPDXLicenseMatcher.Test is an intentional part of the test coverage or if this exclusion is deliberate.
Based on the script results, the review comment has identified a critical issue with the test pattern change. The script output confirms that:
-
Four test projects exist in the solution:
- NuGetLicense.Test
- NuGetUtility.Test
- NuGetUtility.UrlToLicenseMapping.Test
- SPDXLicenseMatcher.Test
-
The new pattern
NuGet*.Test.dllwill exclude SPDXLicenseMatcher.Test because it doesn't start with the "NuGet" prefix. -
The old pattern
*.Test.*dllwould have included all four test assemblies, providing complete coverage.
This is a definite regression in test coverage that will cause SPDXLicenseMatcher.Test.dll to be excluded from the release workflow testing.
Update test assembly pattern to include all test projects.
The change from *.Test.*dll to NuGet*.Test.dll excludes SPDXLicenseMatcher.Test.dll, reducing test coverage. Update the pattern to *Test*.dll or explicitly list all test projects to ensure comprehensive test execution in the release workflow.
🤖 Prompt for AI Agents
.github/workflows/release.yml around line 38: the testAssembly pattern was
narrowed to "NuGet*.Test.dll" which excludes SPDXLicenseMatcher.Test.dll and
reduces test coverage; change the value to a pattern that includes all test
assemblies (for example "*Test*.dll") or replace with an explicit
comma-separated list of all four test DLLs (NuGetLicense.Test.dll,
NuGetUtility.Test.dll, NuGetUtility.UrlToLicenseMapping.Test.dll,
SPDXLicenseMatcher.Test.dll) so the release workflow runs every test project.
There was a problem hiding this comment.
The exclusion is deliberate
There was a problem hiding this comment.
@sensslen, understood! I've noted that the exclusion of SPDXLicenseMatcher.Test.dll from the release workflow is intentional. Thank you for the clarification.
✏️ Learnings added
Learnt from: sensslen
Repo: sensslen/nuget-license PR: 327
File: .github/workflows/release.yml:38-38
Timestamp: 2025-11-07T21:33:27.152Z
Learning: In the release workflow (.github/workflows/release.yml), the test assembly pattern `NuGet*.Test.dll` intentionally excludes SPDXLicenseMatcher.Test.dll from the release workflow tests.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
sensslen
force-pushed
the
simon/fix-release-pipeline
branch
from
b69736b to
5ecb86f
Compare
|
1 participant
Summary by CodeRabbit
Chores
Tests