Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adding hardware test #351

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Adding hardware test #351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
b605096
Adding hardware test
Ellerbach Oct 14, 2024
20ed2bc
updating benchmark packages
Ellerbach Oct 14, 2024
d0c1ab2
Merge branch 'main' into add-hardware-tests
Ellerbach Oct 14, 2024
00f9e1d
adjusting to point on template
Ellerbach Oct 14, 2024
679e46b
Merge branch 'add-hardware-tests' of https://github.com/nanoframework…
Ellerbach Oct 14, 2024
bd361a0
adjusting paths
Ellerbach Oct 14, 2024
5af0ba5
adjusting test names
Ellerbach Oct 14, 2024
a982d02
adjusting for Unix based file name
Ellerbach Oct 14, 2024
1962af4
Merge branch 'main' into add-hardware-tests
josesimoes Oct 14, 2024
b9ab875
asjusting for new adapter and path
Ellerbach Oct 16, 2024
2aa239a
Merge branch 'add-hardware-tests' of https://github.com/nanoframework…
Ellerbach Oct 16, 2024
9041b38
adjusting runsettings
Ellerbach Oct 16, 2024
7e7818d
adding boostrap
Ellerbach Oct 31, 2024
727367b
Merge branch 'main' into add-hardware-tests
Ellerbach Oct 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 169 additions & 0 deletions azure-bootstrap.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
# Copyright (c) .NET Foundation and Contributors
# See LICENSE file in the project root for full license information.

trigger:
branches:
include:
- main
- develop
- release-*
paths:
exclude:
- .github_changelog_generator
- .gitignore
- CHANGELOG.md
- CODE_OF_CONDUCT.md
- LICENSE.md
- README.md
- NuGet.Config
- assets/*
- config/*
- .github/*

# PR always trigger build
pr:
autoCancel: true

jobs:
- job: Trigger
displayName: Trigger Azure Dev Ops build and test pipeline
pool:
vmImage: 'ubuntu-latest'

variables:
AZURE_DEVOPS_ORG: nanoFramework
AZURE_DEVOPS_PROJECT: nanoFramework.Json
AZURE_DEVOPS_PIPELINE_ID: 59
AZURE_POOL_NAME: TestStream

Comment on lines +33 to +38
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider parameterizing pipeline configuration.

The hardcoded values for organization, project, pipeline ID, and pool name reduce reusability. Consider moving these to pipeline variables or parameters that can be configured at runtime.

parameters:
  - name: azureDevOpsOrg
    type: string
    default: nanoFramework
  - name: azureDevOpsProject
    type: string
    default: nanoFramework.Json
  - name: pipelineId
    type: number
    default: 59
  - name: poolName
    type: string
    default: TestStream

variables:
  AZURE_DEVOPS_ORG: ${{ parameters.azureDevOpsOrg }}
  AZURE_DEVOPS_PROJECT: ${{ parameters.azureDevOpsProject }}
  AZURE_DEVOPS_PIPELINE_ID: ${{ parameters.pipelineId }}
  AZURE_POOL_NAME: ${{ parameters.poolName }}
🧰 Tools
🪛 yamllint

[error] 37-37: trailing spaces

(trailing-spaces)

steps:
- script: |
# Validate required environment variables
for var in AZURE_DEVOPS_ORG AZURE_DEVOPS_PROJECT AZURE_DEVOPS_PIPELINE_ID AZURE_POOL_NAME; do
if [ -z "${!var}" ]; then
echo "Error: Required environment variable $var is not set"
exit 1
fi
done

# Define the Azure DevOps organization, project, and pipeline
organization="${AZURE_DEVOPS_ORG}"
project="${AZURE_DEVOPS_PROJECT}"
pipelineId="${AZURE_DEVOPS_PIPELINE_ID}"
poolName="${AZURE_POOL_NAME}"
branch="${BUILD_SOURCEBRANCH}"

# Encode the PAT
patEncoded=$(echo -n ":${AZURE_DEVOPS_PAT}" | base64)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Security: Avoid base64 encoding PAT in script.

Base64 encoding the PAT in the script makes it vulnerable to logging. Consider using Azure DevOps predefined variables or a secure task for authentication.

-      patEncoded=$(echo -n ":${AZURE_DEVOPS_PAT}" | base64)
+      # Use System.AccessToken or az devops login

Committable suggestion was skipped due to low confidence.


# Define the headers
headers=(
-H "Authorization: Basic $patEncoded"
-H "Content-Type: application/json"
)

# Get the pool ID
url="https://dev.azure.com/${organization}/_apis/distributedtask/pools?poolName=${poolName}&api-version=7.1"
AZP_POOL_AGENTS=$(curl -s "${headers[@]}" -X GET "$url")
poolId=$(echo "$AZP_POOL_AGENTS" | jq -r '.value[0].id')

echo "Pool ID: $poolId"

# Define the URL to get all agents in the pool
url="https://dev.azure.com/${organization}/_apis/distributedtask/pools/${poolId}/agents?includeCapabilities=true&api-version=7.1"

response=$(curl -s -w "%{http_code}" "${headers[@]}" -X GET "$url")
http_code=${response: -3}
content=${response::-3}

if [ $http_code -eq 200 ]; then
# Extract all userCapabilities names for online and enabled agents as a unique list
capabilityNames=$(echo "$content" | jq -r '[.value[] | select(.status == "online" and .enabled == true) | .userCapabilities | keys] | unique | flatten | join("\n- ")')
else
echo "Failed to retrieve agent capabilities. HTTP Status Code: $http_code"
echo "Response: \"$content\""
exit 1
fi
Comment on lines +65 to +86
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add timeout and retry mechanism for API calls.

The API calls to fetch pool information lack timeout and retry mechanisms, which could lead to pipeline hangs or failures due to transient issues.

+      # Function to retry API calls
+      function retry_curl() {
+        local retries=3
+        local wait=5
+        local timeout=30
+        local i=0
+        while [ $i -lt $retries ]; do
+          response=$(curl -s -w "%{http_code}" --connect-timeout $timeout "${headers[@]}" "$@")
+          if [ $? -eq 0 ]; then
+            return 0
+          fi
+          i=$((i+1))
+          [ $i -lt $retries ] && sleep $wait
+        done
+        return 1
+      }
+
       # Get the pool ID
       url="https://dev.azure.com/${organization}/_apis/distributedtask/pools?poolName=${poolName}&api-version=7.1"
-      AZP_POOL_AGENTS=$(curl -s "${headers[@]}" -X GET "$url")
+      if ! retry_curl -X GET "$url"; then
+        echo "Failed to get pool ID after retries"
+        exit 1
+      fi

Committable suggestion was skipped due to low confidence.

echo "Unique userCapabilities names: \"$capabilityNames\""

# Prepare the parameters
parametersJson=$(jq -n --arg appComponents "- $capabilityNames" '{templateParameters: {appComponents: $appComponents}}')

echo "Parameters: \"$parametersJson\""
echo "Branch for PR: \"$branch\""

# Define the request body
bodyJson=$(jq -n --argjson parameters "$parametersJson" --arg branch "$branch" '{
resources: {
repositories:
{
self: {
refName: $branch
}
}
},
templateParameters: $parameters.templateParameters
}')

echo "Request body: \"$bodyJson\""

# Define the URL
url="https://dev.azure.com/${organization}/${project}/_apis/pipelines/${pipelineId}/runs?api-version=7.1"

# Trigger the pipeline
response=$(curl -s -w "%{http_code}" "${headers[@]}" -X POST -d "$bodyJson" "$url")
http_code=${response: -3}
content=${response::-3}

if [ $http_code -eq 200 ]; then
run_id=$(echo "$content" | jq -r '.id')
echo "Pipeline triggered successfully. Run ID: $run_id"
echo "##vso[task.setvariable variable=run_id]$run_id"
else
echo "Failed to trigger pipeline. HTTP Status Code: $http_code"
echo "Response: $content"
exit 1
fi
displayName: 'Trigger Azure DevOps Pipeline'
env:
BUILD_SOURCEBRANCH: $(Build.SourceBranch)
AZURE_DEVOPS_PAT: $(AZURE_DEVOPS_PAT)

- script: |
echo "Pipeline to monitor Run ID: $(run_id)"
# Define the URL to get the pipeline run status
url="https://dev.azure.com/${AZURE_DEVOPS_ORG}/${AZURE_DEVOPS_PROJECT}/_apis/pipelines/${AZURE_DEVOPS_PIPELINE_ID}/runs/$(run_id)?api-version=7.1"

# Loop to monitor the pipeline run status
while true; do
response=$(curl -s -w "%{http_code}" -H "Authorization: Basic $(echo -n ":${AZURE_DEVOPS_PAT}" | base64)" -X GET "$url")
http_code=${response: -3}
content=${response::-3}

if [ $http_code -eq 200 ]; then
state=$(echo "$content" | jq -r '.state')
result=$(echo "$content" | jq -r '.result')

echo "Pipeline run state: $state"

if [ "$state" == "completed" ]; then
echo "Pipeline run completed with result: $result"
if [ "$result" == "succeeded" ]; then
exit 0
else
exit 1
fi
fi
else
echo "Failed to get pipeline run status. HTTP Status Code: $http_code"
echo "Response: $content"
exit 1
fi

# Wait for a while before checking again
sleep 30
done
Comment on lines +132 to +165
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve pipeline monitoring robustness.

The current monitoring implementation has several potential issues:

  1. No maximum wait time could lead to infinite execution
  2. Fixed sleep interval isn't optimal for long-running tests
  3. Basic error handling could miss edge cases
+      # Configuration
+      max_wait_time=7200  # 2 hours
+      start_time=$SECONDS
+      initial_sleep=30
+      max_sleep=300  # 5 minutes
+      current_sleep=$initial_sleep
+
       # Loop to monitor the pipeline run status
       while true; do
+          # Check if maximum wait time exceeded
+          elapsed=$(( SECONDS - start_time ))
+          if [ $elapsed -gt $max_wait_time ]; then
+              echo "Pipeline monitoring timed out after $(( max_wait_time / 60 )) minutes"
+              exit 1
+          fi
+
           response=$(curl -s -w "%{http_code}" -H "Authorization: Basic $(echo -n ":${AZURE_DEVOPS_PAT}" | base64)" -X GET "$url")
           http_code=${response: -3}
           content=${response::-3}
 
           if [ $http_code -eq 200 ]; then
               state=$(echo "$content" | jq -r '.state')
               result=$(echo "$content" | jq -r '.result')
 
               echo "Pipeline run state: $state"
 
               if [ "$state" == "completed" ]; then
                   echo "Pipeline run completed with result: $result"
                   if [ "$result" == "succeeded" ]; then
                       exit 0
                   else
                       exit 1
                   fi
+              elif [ "$state" == "canceling" ] || [ "$state" == "canceled" ]; then
+                  echo "Pipeline was canceled"
+                  exit 1
               fi
           else
               echo "Failed to get pipeline run status. HTTP Status Code: $http_code"
               echo "Response: $content"
               exit 1
           fi
 
-          # Wait for a while before checking again
-          sleep 30
+          # Exponential backoff with maximum limit
+          sleep $current_sleep
+          current_sleep=$(( current_sleep * 2 ))
+          [ $current_sleep -gt $max_sleep ] && current_sleep=$max_sleep
       done
📝 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.

Suggested change
- script: |
echo "Pipeline to monitor Run ID: $(run_id)"
# Define the URL to get the pipeline run status
url="https://dev.azure.com/${AZURE_DEVOPS_ORG}/${AZURE_DEVOPS_PROJECT}/_apis/pipelines/${AZURE_DEVOPS_PIPELINE_ID}/runs/$(run_id)?api-version=7.1"
# Loop to monitor the pipeline run status
while true; do
response=$(curl -s -w "%{http_code}" -H "Authorization: Basic $(echo -n ":${AZURE_DEVOPS_PAT}" | base64)" -X GET "$url")
http_code=${response: -3}
content=${response::-3}
if [ $http_code -eq 200 ]; then
state=$(echo "$content" | jq -r '.state')
result=$(echo "$content" | jq -r '.result')
echo "Pipeline run state: $state"
if [ "$state" == "completed" ]; then
echo "Pipeline run completed with result: $result"
if [ "$result" == "succeeded" ]; then
exit 0
else
exit 1
fi
fi
else
echo "Failed to get pipeline run status. HTTP Status Code: $http_code"
echo "Response: $content"
exit 1
fi
# Wait for a while before checking again
sleep 30
done
- script: |
echo "Pipeline to monitor Run ID: $(run_id)"
# Define the URL to get the pipeline run status
url="https://dev.azure.com/${AZURE_DEVOPS_ORG}/${AZURE_DEVOPS_PROJECT}/_apis/pipelines/${AZURE_DEVOPS_PIPELINE_ID}/runs/$(run_id)?api-version=7.1"
# Configuration
max_wait_time=7200 # 2 hours
start_time=$SECONDS
initial_sleep=30
max_sleep=300 # 5 minutes
current_sleep=$initial_sleep
# Loop to monitor the pipeline run status
while true; do
# Check if maximum wait time exceeded
elapsed=$(( SECONDS - start_time ))
if [ $elapsed -gt $max_wait_time ]; then
echo "Pipeline monitoring timed out after $(( max_wait_time / 60 )) minutes"
exit 1
fi
response=$(curl -s -w "%{http_code}" -H "Authorization: Basic $(echo -n ":${AZURE_DEVOPS_PAT}" | base64)" -X GET "$url")
http_code=${response: -3}
content=${response::-3}
if [ $http_code -eq 200 ]; then
state=$(echo "$content" | jq -r '.state')
result=$(echo "$content" | jq -r '.result')
echo "Pipeline run state: $state"
if [ "$state" == "completed" ]; then
echo "Pipeline run completed with result: $result"
if [ "$result" == "succeeded" ]; then
exit 0
else
exit 1
fi
elif [ "$state" == "canceling" ] || [ "$state" == "canceled" ]; then
echo "Pipeline was canceled"
exit 1
fi
else
echo "Failed to get pipeline run status. HTTP Status Code: $http_code"
echo "Response: $content"
exit 1
fi
# Exponential backoff with maximum limit
sleep $current_sleep
current_sleep=$(( current_sleep * 2 ))
[ $current_sleep -gt $max_sleep ] && current_sleep=$max_sleep
done

displayName: 'Monitoring Azure DevOps pipeline'
env:
run_id: $(run_id)
AZURE_DEVOPS_PAT: $(AZURE_DEVOPS_PAT)
85 changes: 48 additions & 37 deletions azure-pipelines.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,10 @@
# Copyright (c) .NET Foundation and Contributors
# See LICENSE file in the project root for full license information.

trigger:
branches:
include:
- main
- develop
- release-*
paths:
exclude:
- .github_changelog_generator
- .gitignore
- CHANGELOG.md
- CODE_OF_CONDUCT.md
- LICENSE.md
- README.md
- NuGet.Config
- assets/*
- config/*
- .github/*

# PR always trigger build
pr:
autoCancel: true
# The Pipeline is going to be called by the GitHub action.
# Manual trigger is always possible.
trigger: none
pr: none
Comment on lines +4 to +7
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

⚠️ Missing GitHub Action integration for Azure Pipelines

The pipeline is configured to be triggered by a GitHub Action, but no workflow files currently integrate with Azure Pipelines. This could lead to the pipeline never being triggered as intended.

  • .github/workflows/pr-checks.yml only runs package checks
  • .github/workflows/update-dependencies.yml only handles dependency updates
  • No workflow file contains integration with azure-pipelines.yml
🔗 Analysis chain

LGTM! Verify GitHub Action integration.

The trigger configuration is correct for external invocation. The comments clearly explain the pipeline's execution context.

Let's verify the GitHub Action integration:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for GitHub Action workflow files that trigger this pipeline
fd -e yml -e yaml . .github/workflows --exec grep -l "azure-pipelines"

Length of output: 70

Script:

#!/bin/bash
# Let's try a broader search for GitHub workflow files and Azure Pipeline references
# First, find all workflow files
echo "=== GitHub Workflow Files ==="
fd -e yml -e yaml . .github/workflows

# Then, search for any Azure Pipeline related patterns in yaml/yml files
echo -e "\n=== Files referencing Azure Pipelines ==="
rg -l -g "*.{yml,yaml}" -i "azure.?pipeline|azure.?devops" .

Length of output: 388

Script:

#!/bin/bash
# Let's check the content of pr-checks.yml to see if it triggers azure-pipelines
echo "=== Content of pr-checks.yml ==="
cat .github/workflows/pr-checks.yml

# Also check update-dependencies.yml just to be thorough
echo -e "\n=== Content of update-dependencies.yml ==="
cat .github/workflows/update-dependencies.yml

Length of output: 1366


# add nf-tools repo to resources (for Azure Pipelines templates)
resources:
Expand All @@ -48,20 +30,49 @@ variables:
- name: nugetPackageName
value: 'nanoFramework.Json'

steps:
parameters:
- name: appComponents
displayName: List of capabilities to run the tests on
type: object
default:
- none

Comment on lines +33 to +39
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Improve parameter configuration and validation.

The default value of none for appComponents could lead to scenarios where no tests are executed, which aligns with the reported issue in PR comments about catching missing tests.

Consider these improvements:

  1. Add parameter validation to prevent empty or 'none' values
  2. Enhance the parameter description to list valid component options
  3. Add a minimum number of required components

Here's a suggested improvement:

 parameters:
 - name: appComponents
   displayName: List of capabilities to run the tests on
   type: object
+  # List of valid components: [component1, component2, ...]
+  # At least one component must be specified
   default:
-    - none
+    - required: true
+    - allowed:
+      - component1
+      - component2

Committable suggestion was skipped due to low confidence.

🧰 Tools
🪛 yamllint

[warning] 34-34: wrong indentation: expected 2 but found 0

(indentation)

stages:
- stage: Build
displayName: 'Build'
jobs:
- job: Build
displayName: 'Build job'
pool:
# default is the following VM Image
vmImage: 'windows-latest'
steps:

# step from template @ nf-tools repo
# all build, update and publish steps
- template: azure-pipelines-templates/class-lib-build.yml@templates
parameters:
sonarCloudProject: 'nanoframework_lib-nanoFramework.Json'
runUnitTests: true
unitTestRunsettings: '$(System.DefaultWorkingDirectory)\.runsettings'

# step from template @ nf-tools repo
# report error
- template: azure-pipelines-templates/discord-webhook-task.yml@templates
parameters:
status: 'failure'
webhookUrl: '$(DiscordWebhook)'
message: ''

# step from template @ nf-tools repo
# all build, update and publish steps
- template: azure-pipelines-templates/class-lib-build.yml@templates
parameters:
sonarCloudProject: 'nanoframework_lib-nanoFramework.Json'
runUnitTests: true
unitTestRunsettings: '$(System.DefaultWorkingDirectory)\.runsettings'
- task: PublishPipelineArtifact@1
displayName: Publish Pipeline Artifact copy
inputs:
path: '$(System.DefaultWorkingDirectory)'
artifactName: 'Artifacts'

# step from template @ nf-tools repo
# report error
- template: azure-pipelines-templates/discord-webhook-task.yml@templates
parameters:
status: 'failure'
webhookUrl: '$(DiscordWebhook)'
message: ''
- ${{ each appComponents in parameters.appComponents }}:
- template: azure-pipelines-templates/device-test.yml@templates
parameters:
appComponents: ${{ appComponents }}
unitTestRunsettings:
- 'nanoFramework.Json.Test/nano.runsettings,nanoFramework.Json.Test/bin/Release/NFUnitTest.dll'
Comment on lines +73 to +78
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Improve cross-platform compatibility and test validation.

The device test configuration needs improvements to address the path handling issues mentioned in PR comments:

  1. Use forward slashes for cross-platform compatibility
  2. Add test file existence validation
  3. Remove trailing spaces

Apply these changes:

 - ${{ each appComponents in parameters.appComponents }}:
     - template: azure-pipelines-templates/device-test.yml@templates
       parameters:
         appComponents: ${{ appComponents }}
-        unitTestRunsettings: 
-          - 'nanoFramework.Json.Test/nano.runsettings,nanoFramework.Json.Test/bin/Release/NFUnitTest.dll'
+        unitTestRunsettings:
+          - 'nanoFramework.Json.Test/nano.runsettings,nanoFramework.Json.Test/bin/$(buildConfiguration)/NFUnitTest.dll'

Add a validation step before running tests:

- task: PowerShell@2
  displayName: 'Validate Test Files'
  inputs:
    targetType: 'inline'
    script: |
      $testPath = "nanoFramework.Json.Test/bin/$(buildConfiguration)/NFUnitTest.dll"
      if (-not (Test-Path $testPath)) {
        Write-Error "Test assembly not found: $testPath"
        exit 1
      }
🧰 Tools
🪛 yamllint

[error] 73-73: trailing spaces

(trailing-spaces)

[warning] 74-74: wrong indentation: expected 4 but found 2

(indentation)

[error] 77-77: trailing spaces

(trailing-spaces)

[error] 78-78: no new line character at the end of file

(new-line-at-end-of-file)

1 change: 1 addition & 0 deletions nanoFramework.Json.Test/nano.runsettings
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,6 @@
<nanoFrameworkAdapter>
<Logging>None</Logging>
<IsRealHardware>False</IsRealHardware>
<RealHardwarePort>COM3</RealHardwarePort>
</nanoFrameworkAdapter>
</RunSettings>
Loading