Update appveyor.yml #373
Update appveyor.yml #373
Conversation
WalkthroughThe changes in this pull request focus on restructuring the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
the
size/L
gstraccini
bot
added
the
☑️ auto-merge
![gstraccini[bot]](https://avatars.githubusercontent.com/in/480132?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (7)
appveyor.yml (7)
37-42: Consider adding token validation for enhanced securityThe PR detection logic is good, but consider adding token validation to ensure the security tokens are properly set before attempting to use them.
- ps: $env:LOCAL_PR = 0 - ps: if(-Not $env:APPVEYOR_PULL_REQUEST_NUMBER) { $env:LOCAL_PR = 1 } - ps: if($env:APPVEYOR_PULL_REQUEST_HEAD_REPO_NAME -Eq $env:APPVEYOR_REPO_NAME) { $env:LOCAL_PR = 1 } +- ps: | + if ($env:LOCAL_PR -eq 1) { + if (-Not $env:SONAR_TOKEN) { throw "SONAR_TOKEN is not set" } + if (-Not $env:CODACY_PROJECT_TOKEN) { throw "CODACY_PROJECT_TOKEN is not set" } + if (-Not $env:CODECLIMATE_TOKEN) { throw "CODECLIMATE_TOKEN is not set" } + }
67-71: Fix trailing whitespace and consider parameterizing .NET version
- Remove trailing whitespace on line 70
- Consider parameterizing the .NET version in coverage paths to make it more maintainable
- $wc.UploadFile("https://ci.appveyor.com/api/testresults/xunit/$($env:APPVEYOR_JOB_ID)", (Resolve-Path $path/test-results.xml)) + $wc.UploadFile("https://ci.appveyor.com/api/testresults/xunit/$($env:APPVEYOR_JOB_ID)", (Resolve-Path $path/test-results.xml))Also consider adding:
$env:DOTNET_VERSION = "net8.0"And using it in coverage paths:
"/p:CoverletOutputFormat=`"cobertura,opencover,lcov`"" --logger:"junit;LogFilePath=test-results.xml"🧰 Tools
🪛 yamllint
[error] 70-70: trailing spaces
(trailing-spaces)
79-80: Enhance exclusions file contentThe exclusions.txt file content seems minimal. Consider expanding it to exclude other common unnecessary files.
-echo refs > exclusions.txt +@echo refs > exclusions.txt +@echo *.pdb >> exclusions.txt +@echo *.xml >> exclusions.txt +@echo *.deps.json >> exclusions.txt
81-82: Consider moving configuration to variables sectionMove these arrays to the environment section for better maintainability and reusability.
- - ps: $VERSIONS = ("netstandard2.0", "netstandard2.1", "net6.0", "net8.0") - - ps: $PROJECTS = ("Health") +environment: + VERSIONS: netstandard2.0;netstandard2.1;net6.0;net8.0 + PROJECTS: HealthThen use:
$VERSIONS = $env:VERSIONS.Split(';') $PROJECTS = $env:PROJECTS.Split(';')
129-141: Consider adding deployment validation stepsThe deployment configuration looks good, but consider adding validation steps to ensure package quality before deployment.
Add these checks before deployment:
- provider: NuGet api_key: $(NUGET_TOKEN) skip_symbols: false artifact: /.*\.nupkg/ on: branch: main APPVEYOR_REPO_TAG: true # Only deploy on tagged commits
Line range hint
1-141: Add pipeline documentationConsider adding comments to document the pipeline's major sections and their purposes. This will help future maintainers understand the workflow.
Add section headers like:
# Build configuration version: 2.3.{build} skip_tags: true # Build environment setup environment: # Tokens for various services CODACY_PROJECT_TOKEN: secure: ... # And so on for each major section🧰 Tools
🪛 yamllint
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 70-70: trailing spaces
(trailing-spaces)
Line range hint
6-8: Document security token managementConsider adding documentation about token rotation procedures and access requirements for the secure tokens.
Add a comment block:
environment: # Security tokens - Rotate every 90 days # Required permissions: # - CODACY_PROJECT_TOKEN: Project-level access # - CODECLIMATE_TOKEN: Repository-level access CODACY_PROJECT_TOKEN: secure: ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
appveyor.yml(2 hunks)
🧰 Additional context used
🪛 yamllint
appveyor.yml
[error] 70-70: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
appveyor.yml (1)
22-23: LGTM: Java environment setup for coverage tools
The Java environment setup is correctly configured for the Codacy coverage reporter.
|
❌ Build VTEX-SDK-dotnet 2.3.1687 failed (commit e9205e43d3 by @gstraccini[bot]) |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
appveyor.yml (4)
44-47: Consider consolidating coverage toolsThe configuration installs multiple coverage reporting tools (OpenCover, Codecov, Codacy, CodeClimate) which might be redundant. Consider standardizing on fewer tools to reduce build complexity and maintenance overhead.
67-71: Fix test results upload formatting
- Remove trailing spaces on line 70
- Consider adding error handling for test result upload
- $wc.UploadFile("https://ci.appveyor.com/api/testresults/xunit/$($env:APPVEYOR_JOB_ID)", (Resolve-Path $path/test-results.xml)) + try { + $wc.UploadFile("https://ci.appveyor.com/api/testresults/xunit/$($env:APPVEYOR_JOB_ID)", (Resolve-Path $path/test-results.xml)) + } catch { + Write-Warning "Failed to upload test results: $_" + }🧰 Tools
🪛 yamllint
[error] 70-70: trailing spaces
(trailing-spaces)
81-82: Consider dynamic version detectionInstead of hardcoding supported .NET versions and projects, consider detecting them from the solution/project files.
# Example approach for version detection $VERSIONS = Get-ChildItem -Path Src -Filter *.csproj | Select-String -Pattern "<TargetFramework>(.*)</TargetFramework>" | ForEach-Object { $_.Matches.Groups[1].Value } | Sort-Object -Unique
Line range hint
1-141: Addressing the reported build failureBased on the review, here are the key areas to investigate for the build failure:
- Verify Java 19 installation on the build agent
- Check if the hardcoded paths for coverage reports match the actual file locations
- Consider simplifying the coverage tool chain to reduce potential points of failure
Would you like me to help investigate the build logs for more specific error details?
🧰 Tools
🪛 yamllint
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 70-70: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
appveyor.yml(3 hunks)
🧰 Additional context used
🪛 yamllint
appveyor.yml
[error] 70-70: trailing spaces
(trailing-spaces)
🔇 Additional comments (3)
appveyor.yml (3)
1-1: Version bump looks appropriate
The version increment from 2.3 to 2.4 aligns with the significant changes in the build configuration.
129-141: Deployment configuration looks good
The deployment setup follows best practices:
- Restricted to main branch
- Includes both NuGet and GitHub releases
- Uses secure token handling
22-23: Verify Java installation on build agent
Given the reported build failure, ensure that JDK19 is available on the Visual Studio 2022 image. Consider using a more flexible Java version detection approach.
| - ps: $Params += "/d:sonar.host.url=`"https://sonarcloud.io`" /d:sonar.scanner.scanAll=false" | ||
| - ps: if($env:LOCAL_PR -Eq 1) { $Params += "/d:sonar.token=`"$env:SONAR_TOKEN`"" } | ||
| - ps: $Params += "/d:sonar.exclusions=`"**/bin/**/*,**/obj/**/*`"", "/d:sonar.coverage.exclusions=`"**/$env:SOLUTION_NAME.Tests/**,**/*Tests.cs`"" | ||
| - ps: $Params += "/d:sonar.cs.opencover.reportsPaths=`"Tests\$env:SOLUTION_NAME.Tests\coverage.net8.0.opencover.xml`"" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid hardcoding .NET version in coverage path
The coverage path is hardcoded to net8.0. Consider making it dynamic to support multiple .NET versions.
-/d:sonar.cs.opencover.reportsPaths=`"Tests\$env:SOLUTION_NAME.Tests\coverage.net8.0.opencover.xml`""
+/d:sonar.cs.opencover.reportsPaths=`"Tests\$env:SOLUTION_NAME.Tests\coverage.*.opencover.xml`""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - ps: $Params += "/d:sonar.cs.opencover.reportsPaths=`"Tests\$env:SOLUTION_NAME.Tests\coverage.net8.0.opencover.xml`"" | |
| - ps: $Params += "/d:sonar.cs.opencover.reportsPaths=`"Tests\$env:SOLUTION_NAME.Tests\coverage.*.opencover.xml`"" |
|
@gstraccini appveyor build pr |
|
@gstraccini appveyor build commit |
|
I'm sorry @guibranco, I can't do that. Invalid parameter. 🥺 |
|
@gstraccini appveyor build pull request |
|
AppVeyor build (commit) started! 🚀 Build ID: 50978504 |
|
AppVeyor build (pull request) started! 🚀 Build ID: 50978506 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
appveyor.yml (4)
22-23: Consider using a more resilient Java setupHardcoding the Java path might break if AppVeyor's image changes. Consider using AppVeyor's built-in Java configuration or verifying the path exists.
- - SET JAVA_HOME=C:\Program Files\Java\jdk19 - - SET PATH=%JAVA_HOME%\bin;%PATH% + - ps: | + $javaHome = Get-ChildItem "C:\Program Files\Java" -Filter "jdk*" | Sort-Object Name -Descending | Select-Object -First 1 + if ($javaHome) { + $env:JAVA_HOME = $javaHome.FullName + $env:PATH = "$env:JAVA_HOME\bin;$env:PATH" + } else { + throw "Java installation not found" + }
70-70: Remove trailing spacesRemove trailing spaces to maintain consistent formatting.
- $wc.UploadFile("https://ci.appveyor.com/api/testresults/xunit/$($env:APPVEYOR_JOB_ID)", (Resolve-Path $path/test-results.xml)) + $wc.UploadFile("https://ci.appveyor.com/api/testresults/xunit/$($env:APPVEYOR_JOB_ID)", (Resolve-Path $path/test-results.xml))🧰 Tools
🪛 yamllint
[error] 70-70: trailing spaces
(trailing-spaces)
81-82: Consider externalizing configurationMove version and project lists to environment variables or a configuration file for easier maintenance.
- - ps: $VERSIONS = ("netstandard2.0", "netstandard2.1", "net6.0", "net8.0") - - ps: $PROJECTS = ("Health") + - ps: | + $versionsJson = Get-Content "build-config.json" | ConvertFrom-Json + $VERSIONS = $versionsJson.targetFrameworks + $PROJECTS = $versionsJson.projects
129-134: Enhance deployment reliabilityConsider adding timeout and retry settings for NuGet deployment to handle temporary network issues.
- provider: NuGet api_key: $(NUGET_TOKEN) skip_symbols: false + max_trials: 3 + timeout: 300 on: branch: main
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
appveyor.yml(3 hunks)
🧰 Additional context used
🪛 yamllint
appveyor.yml
[error] 70-70: trailing spaces
(trailing-spaces)
| - cmd: choco install opencover.portable | ||
| - cmd: choco install codecov | ||
| - cmd: curl -L https://github.com/codacy/codacy-coverage-reporter/releases/latest/download/codacy-coverage-reporter-assembly.jar > ./codacy-test-reporter.jar |
There was a problem hiding this comment.
Enhance security and reliability of tool installation
- Specify versions for Chocolatey packages to ensure reproducible builds
- Verify downloaded files' integrity using checksums
- - cmd: choco install opencover.portable
- - cmd: choco install codecov
- - cmd: curl -L https://github.com/codacy/codacy-coverage-reporter/releases/latest/download/codacy-coverage-reporter-assembly.jar > ./codacy-test-reporter.jar
+ - cmd: choco install opencover.portable --version=4.7.1922
+ - cmd: choco install codecov --version=1.13.0
+ - ps: |
+ $codacyUrl = "https://github.com/codacy/codacy-coverage-reporter/releases/latest/download/codacy-coverage-reporter-assembly.jar"
+ $expectedHash = "SHA256_HASH_HERE" # Replace with actual hash
+ Invoke-WebRequest -Uri $codacyUrl -OutFile "./codacy-test-reporter.jar"
+ $actualHash = (Get-FileHash -Path "./codacy-test-reporter.jar" -Algorithm SHA256).Hash
+ if ($actualHash -ne $expectedHash) {
+ throw "Codacy reporter JAR checksum verification failed"
+ }Committable suggestion skipped: line range outside the PR's diff.
| assembly_version: '{version}' | ||
| file_version: '{version}' | ||
| informational_version: '{version}' | ||
| file: '**\*.props' |
There was a problem hiding this comment.
💡 Codebase verification
Changing the file pattern to **/*.props excludes all .csproj files, which may prevent the CI/CD pipeline from building the projects correctly.
🔗 Analysis chain
**Verify the impact of changing file pattern from .csproj to .props
This change could affect version patching behavior. Ensure that all necessary files are still being properly versioned.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for both .props and .csproj files to understand the impact
echo "Checking for .props files:"
fd -e props
echo -e "\nChecking for .csproj files:"
fd -e csproj
Length of output: 335
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more Footnotes
|
|
❌ Build VTEX-SDK-dotnet 2.4.1 failed (commit f39cd63186 by @guibranco) |
|
Infisical secrets check: ✅ No secrets leaked! 💻 Scan logs2:27PM INF scanning for exposed secrets...
2:27PM INF 576 commits scanned.
2:27PM INF scan completed in 302ms
2:27PM INF no leaks found
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
appveyor.yml (3)
70-70: Fix trailing whitespaceRemove trailing spaces at the end of line 70.
- $wc.UploadFile("https://ci.appveyor.com/api/testresults/xunit/$($env:APPVEYOR_JOB_ID)", (Resolve-Path $path/test-results.xml)) + $wc.UploadFile("https://ci.appveyor.com/api/testresults/xunit/$($env:APPVEYOR_JOB_ID)", (Resolve-Path $path/test-results.xml))🧰 Tools
🪛 yamllint
[error] 70-70: trailing spaces
(trailing-spaces)
67-71: Consider parameterizing test result configurationThe test results configuration is hardcoded. Consider making it more configurable:
- Test result filename could be an environment variable
- Test framework type (xunit) could be configurable
- dotnet test $testProject --no-build --verbosity minimal /p:CollectCoverage=true "/p:CoverletOutputFormat=\`"cobertura,opencover,lcov\`"" --logger:"junit;LogFilePath=test-results.xml" + $testResultFile = "test-results.xml" + $testFramework = $env:TEST_FRAMEWORK ?? "xunit" + dotnet test $testProject --no-build --verbosity minimal /p:CollectCoverage=true "/p:CoverletOutputFormat=\`"cobertura,opencover,lcov\`"" --logger:"junit;LogFilePath=$testResultFile" - $wc.UploadFile("https://ci.appveyor.com/api/testresults/xunit/$($env:APPVEYOR_JOB_ID)", (Resolve-Path $path/test-results.xml)) + $wc.UploadFile("https://ci.appveyor.com/api/testresults/$testFramework/$($env:APPVEYOR_JOB_ID)", (Resolve-Path "$path/$testResultFile"))🧰 Tools
🪛 yamllint
[error] 70-70: trailing spaces
(trailing-spaces)
81-82: Consider making framework versions configurableThe .NET versions and project list are hardcoded. Consider moving these to environment variables or a configuration file for better maintainability.
- - ps: $VERSIONS = ("netstandard2.0", "netstandard2.1", "net6.0", "net8.0") - - ps: $PROJECTS = ("Health") + - ps: $VERSIONS = $env:TARGET_FRAMEWORKS?.Split(',') ?? @("netstandard2.0", "netstandard2.1", "net6.0", "net8.0") + - ps: $PROJECTS = $env:SOLUTION_PROJECTS?.Split(',') ?? @("Health")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
appveyor.yml(3 hunks)
🧰 Additional context used
🪛 yamllint
appveyor.yml
[error] 70-70: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
appveyor.yml (2)
22-23: Verify JDK 21 compatibility
The Java runtime has been upgraded from JDK 19 to JDK 21. This is a major version change that could affect tools like SonarQube and Codacy reporter.
#!/bin/bash
# Check for any Java version dependencies in the tools
rg -l "java.*version|jdk.*version|runtime.*version" -g "*.{xml,json,config,props}"1-1: Verify version bump implications
The version has been bumped from 2.3 to 2.4. Please ensure this is intentional and follows semantic versioning principles.
| xcopy Src\$env:SOLUTION_NAME\bin\Release\$version\*.* Build\Core\$version\ /s /f /e /r /k /y /EXCLUDE:exclusions.txt | ||
| foreach($project in $PROJECTS) | ||
| { | ||
| xcopy Src\$env:SOLUTION_NAME.$project\bin\Release\$version\*.* Build\$project\$version\ /s /f /e /r /k /y /EXCLUDE:exclusions.txt | ||
| } |
There was a problem hiding this comment.
Add error handling for file operations
File operations (xcopy, 7z) lack error handling. Consider adding try-catch blocks and validation:
+ - ps: function Copy-ProjectFiles {
+ param($source, $target)
+ if (-not (Test-Path $source)) {
+ Write-Warning "Source path not found: $source"
+ return $false
+ }
+ try {
+ xcopy $source $target /s /f /e /r /k /y /EXCLUDE:exclusions.txt
+ return $true
+ } catch {
+ Write-Error "Failed to copy files from $source to $target: $_"
+ return $false
+ }
+ }
- ps: |
foreach($version in $VERSIONS)
{
- xcopy Src\$env:SOLUTION_NAME\bin\Release\$version\*.* Build\Core\$version\ /s /f /e /r /k /y /EXCLUDE:exclusions.txt
+ $success = Copy-ProjectFiles "Src\$env:SOLUTION_NAME\bin\Release\$version\*.*" "Build\Core\$version\"
+ if (-not $success) { continue }
foreach($project in $PROJECTS)
{
- xcopy Src\$env:SOLUTION_NAME.$project\bin\Release\$version\*.* Build\$project\$version\ /s /f /e /r /k /y /EXCLUDE:exclusions.txt
+ Copy-ProjectFiles "Src\$env:SOLUTION_NAME.$project\bin\Release\$version\*.*" "Build\$project\$version\"
}
}Also applies to: 117-122
| - provider: NuGet | ||
| api_key: $(NUGET_TOKEN) | ||
| skip_symbols: false | ||
| on: | ||
| branch: main | ||
|
|
||
| - provider: GitHub | ||
| on: | ||
| branch: main | ||
| tag: v$(appveyor_build_version) | ||
| description: "Release of $(SOLUTION_NAME) - v$(appveyor_build_version)" | ||
| auth_token: $(GITHUB_TOKEN) | ||
| force_update: true |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding deployment validation
The deployment configuration lacks pre-deployment validation steps. Consider adding checks for required environment variables and artifact existence.
deploy:
- provider: NuGet
api_key: $(NUGET_TOKEN)
skip_symbols: false
+ artifact: /.*\.s?nupkg/
on:
branch: main
+ NUGET_TOKEN: true
- provider: GitHub
on:
branch: main
+ GITHUB_TOKEN: true
tag: v$(appveyor_build_version)
description: "Release of $(SOLUTION_NAME) - v$(appveyor_build_version)"
auth_token: $(GITHUB_TOKEN)
force_update: true
+ artifact: /.*\.(zip|s?nupkg)/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - provider: NuGet | |
| api_key: $(NUGET_TOKEN) | |
| skip_symbols: false | |
| on: | |
| branch: main | |
| - provider: GitHub | |
| on: | |
| branch: main | |
| tag: v$(appveyor_build_version) | |
| description: "Release of $(SOLUTION_NAME) - v$(appveyor_build_version)" | |
| auth_token: $(GITHUB_TOKEN) | |
| force_update: true | |
| - provider: NuGet | |
| api_key: $(NUGET_TOKEN) | |
| skip_symbols: false | |
| artifact: /.*\.s?nupkg/ | |
| on: | |
| branch: main | |
| NUGET_TOKEN: true | |
| - provider: GitHub | |
| on: | |
| branch: main | |
| GITHUB_TOKEN: true | |
| tag: v$(appveyor_build_version) | |
| description: "Release of $(SOLUTION_NAME) - v$(appveyor_build_version)" | |
| auth_token: $(GITHUB_TOKEN) | |
| force_update: true | |
| artifact: /.*\.(zip|s?nupkg)/ |
|
|
✅ Build VTEX-SDK-dotnet 2.4.3 completed (commit ca7c55d08d by @guibranco) |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #373 +/- ##
========================================
- Coverage 0.22% 0.22% -0.01%
========================================
Files 117 117
Lines 1774 1777 +3
Branches 75 75
========================================
Hits 4 4
- Misses 1770 1771 +1
- Partials 0 2 +2 ☔ View full report in Codecov by Sentry. |
Closes #
📑 Description
✅ Checks
☢️ Does this introduce a breaking change?
ℹ Additional Information
Summary by CodeRabbit