Skip to content

Include device logs in test artifacts#169

Open
stefan-silabs wants to merge 12 commits intorelease_2.8-1.5from
include_device_logs_for_ci_tests
Open

Include device logs in test artifacts#169
stefan-silabs wants to merge 12 commits intorelease_2.8-1.5from
include_device_logs_for_ci_tests

Conversation

@stefan-silabs
Copy link
Contributor

@stefan-silabs stefan-silabs commented Oct 29, 2025

Issue Link:
NOJIRA

Description of Problem/Feature:
Include device logs for more information in case of failures.

Description of Fix/Solution:

Testing Done:
CI


Note

Packages pytest report and device logs into a single tar.gz artifact and updates Jenkins test results URL output for PR vs branch builds.

  • Jenkins pipeline (jenkins_integration/jenkinsFunctions.groovy):
    • Artifacts: Replace single pytest-report-*.html upload with test-results-<app>-<board>.tar.gz containing pytest-report.html and any test_tc*.txt logs; archive the tarball.
    • Output: Print tar archive contents for verification.
    • Links: Echo different Jenkins test results URLs for PR (change-requests) vs branch builds.

Written by Cursor Bugbot for commit 8dab24c. This will update automatically on new commits. Configure here.

Copilot AI review requested due to automatic review settings October 29, 2025 16:01
@stefan-silabs stefan-silabs requested a review from a team as a code owner October 29, 2025 16:01
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds device log collection to test artifacts in the Jenkins integration to provide more debugging information when test failures occur. The changes also include debugging commands and placeholder logic for conditional branch-based URL generation.

  • Adds debug commands to list directory contents during test execution
  • Includes a TODO comment for archiving device logs alongside HTML reports
  • Comments out the existing test results URL echo statement and adds conditional logic for different branch types

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings October 30, 2025 13:05
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@@ -157,10 +157,14 @@ def execute_sanity_tests(nomadNode, deviceGroup, deviceGroupId, harnessTemplate,
}
}
}
Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

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

The wildcard pattern test_tc*.txt for device logs is unclear and could match unintended files. Consider using a more specific pattern or documenting what files this is intended to capture.

Suggested change
}
}
// The pattern 'test_tc*.txt' is intended to capture all test case result files generated by the test framework,
// such as 'test_tc01.txt', 'test_tc02.txt', etc. If the naming convention changes, update this pattern accordingly.

Copilot uses AI. Check for mistakes.
Comment on lines +163 to +167
if(branchName.startsWith("PR-")){
echo "See test results here: https://jenkins-cbs-iot-matter.silabs.net/job/Matter%20Extension%20GitHub/view/change-requests/job/${BRANCH_NAME}/${BUILD_NUMBER}/"
} else {
echo "See test results here: https://jenkins-cbs-iot-matter.silabs.net/job/Matter%20Extension%20GitHub/job/${BRANCH_NAME}/${BUILD_NUMBER}/"
}
Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

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

The URLs are duplicated with only a minor difference. Consider extracting the common base URL and conditionally adding the 'view/change-requests' segment to reduce duplication.

Suggested change
if(branchName.startsWith("PR-")){
echo "See test results here: https://jenkins-cbs-iot-matter.silabs.net/job/Matter%20Extension%20GitHub/view/change-requests/job/${BRANCH_NAME}/${BUILD_NUMBER}/"
} else {
echo "See test results here: https://jenkins-cbs-iot-matter.silabs.net/job/Matter%20Extension%20GitHub/job/${BRANCH_NAME}/${BUILD_NUMBER}/"
}
def baseUrl = "https://jenkins-cbs-iot-matter.silabs.net/job/Matter%20Extension%20GitHub"
def changeRequestsSegment = branchName.startsWith("PR-") ? "/view/change-requests" : ""
def testResultsUrl = "${baseUrl}${changeRequestsSegment}/job/${BRANCH_NAME}/${BUILD_NUMBER}/"
echo "See test results here: ${testResultsUrl}"

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings October 31, 2025 17:05
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

}
sh "cp ./reports/pytest-report.html ./reports/pytest-report-${appName}-${board}.html"
archiveArtifacts artifacts: "reports/pytest-report-${appName}-${board}.html"
sh "tar -czf ./reports/test-results-${appName}-${board}.tar.gz -C ./reports pytest-report.html test_tc*.txt || true"
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

The || true at the end of the tar command will suppress all errors, including cases where pytest-report.html doesn't exist. Consider handling missing files more explicitly or at least logging when the tar command fails.

Suggested change
sh "tar -czf ./reports/test-results-${appName}-${board}.tar.gz -C ./reports pytest-report.html test_tc*.txt || true"
// Check if at least one artifact exists before attempting to tar
def pytestReportExists = fileExists('reports/pytest-report.html')
def testTcFiles = sh(script: "ls reports/test_tc*.txt 2>/dev/null | wc -l", returnStdout: true).trim()
if (pytestReportExists || testTcFiles != "0") {
try {
sh "tar -czf ./reports/test-results-${appName}-${board}.tar.gz -C ./reports pytest-report.html test_tc*.txt"
} catch (err) {
echo "Error creating tarball: ${err}"
}
} else {
echo "Warning: No test report artifacts found to archive (pytest-report.html or test_tc*.txt missing)."
}

Copilot uses AI. Check for mistakes.
Rehan Rasool (rerasool) pushed a commit to rerasool/matter_extension that referenced this pull request Nov 7, 2025
…r lock an .slcp

Merge in WMN_TOOLS/matter_extension from bugfix/doorlock_silabs_zap to release_2.5-1.4

Squashed commit of the following:

commit 07c3af80ba8a5e7105527a28004b0c626ffac83f
Author: lpbeliveau-silabs <louis-philip.beliveau@silabs.com>
Date:   Wed Nov 13 15:03:13 2024 -0500

    Updated the Zap file path in our lock an .slcp
@stefan-silabs stefan-silabs changed the base branch from main to release_2.8-1.5 November 24, 2025 14:00
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Bug: Missing run_number parameter in SQA artifact upload

The call to upload_artifacts for SQA artifacts is missing the run_number parameter. While commit_sha and workflow_id are passed, run_number defaults to the string "null". This causes the Python script to receive --run_number null as an argument, which may lead to incorrect behavior since run_number is a required parameter that should contain the actual workflow run number available in the run_number variable.

jenkins_integration/Jenkinsfile#L209-L210

withDockerContainer(image: 'artifactory.silabs.net/gsdk-docker-production/gsdk_nomad_containers/gsdk_ubai:latest') {
def result = pipelineFunctions.upload_artifacts(sqa=true, commit_sha=commit_sha, workflow_id=workflow_id)

Fix in Cursor Fix in Web


@stefan-silabs stefan-silabs reopened this Nov 24, 2025
@stefan-silabs stefan-silabs changed the title (TESTING) Include device logs in test artifacts Include device logs in test artifacts Nov 24, 2025
echo "See test results here: https://jenkins-cbs-iot-matter.silabs.net/job/Matter%20Extension%20GitHub/view/change-requests/job/${BRANCH_NAME}/${BUILD_NUMBER}/"
} else {
echo "See test results here: https://jenkins-cbs-iot-matter.silabs.net/job/Matter%20Extension%20GitHub/job/${BRANCH_NAME}/${BUILD_NUMBER}/"
}
Copy link

Choose a reason for hiding this comment

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

Bug: URL uses wrong variable for branch name

The condition on line 362 checks the branchName parameter to determine if it's a PR build, but the URLs on lines 363 and 365 use the BRANCH_NAME environment variable instead. When branchName is set to PR-{number} format (as passed from the caller), BRANCH_NAME remains the actual Git branch name, causing the generated URLs to point to incorrect Jenkins job paths that won't match the PR naming convention.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants