Add test results artifact uploads to CI workflow - #75
Merged
Conversation
github-actions
Bot
added
the
squad
Contributor
🏗️ PR Added to Squad Triage QueueThis PR has been labeled with Next steps:
|
Contributor
SummarySummary
CoverageAppHost - 0%
Domain - 87.2%
ServiceDefaults - 0%
Web - 68.7%
|
There was a problem hiding this comment.
Pull request overview
This PR aims to improve CI observability by uploading per-suite test result artifacts so test outputs remain available even when a test step fails.
Changes:
- Added
actions/upload-artifactsteps to upload Architecture/Unit/Integration test results directories in.github/workflows/ci.yml. - Added a vendored
dotnet-install.shscript to the repo root. - Added local build output/log files under
docs/build-log.txtandbuild-output.log(plus a minorglobal.jsonformatting change).
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/ci.yml |
Uploads per-suite test result artifacts with 30-day retention and if: always(). |
global.json |
Minor formatting-only change. |
dotnet-install.sh |
Adds a full vendored .NET installer script (currently appears unused by CI). |
docs/build-log.txt |
Adds a machine-specific build/test log (includes absolute paths). |
build-output.log |
Adds machine-specific build output log (includes absolute paths). |
Comment on lines
+90
to
+96
| - name: Upload Architecture Test Results | ||
| uses: actions/upload-artifact@v7 | ||
| if: always() | ||
| with: | ||
| name: architecture-test-results | ||
| path: test-results/architecture | ||
| retention-days: 30 |
There was a problem hiding this comment.
The PR description/issue scope is about adding CI artifact uploads, but this workflow change comes alongside other unrelated additions in the PR (e.g., dotnet-install.sh and build log files). Please either remove the unrelated files from this PR or update the PR description to explain why they’re needed as part of #73.
Comment on lines
+2
to
+137
| # Copyright (c) .NET Foundation and contributors. All rights reserved. | ||
| # Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
| # | ||
|
|
||
| # Stop script on NZEC | ||
| set -e | ||
| # Stop script if unbound variable found (use ${var:-} if intentional) | ||
| set -u | ||
| # By default cmd1 | cmd2 returns exit code of cmd2 regardless of cmd1 success | ||
| # This is causing it to fail | ||
| set -o pipefail | ||
|
|
||
| # Use in the the functions: eval $invocation | ||
| invocation='say_verbose "Calling: ${yellow:-}${FUNCNAME[0]} ${green:-}$*${normal:-}"' | ||
|
|
||
| # standard output may be used as a return value in the functions | ||
| # we need a way to write text on the screen in the functions so that | ||
| # it won't interfere with the return value. | ||
| # Exposing stream 3 as a pipe to standard output of the script itself | ||
| exec 3>&1 | ||
|
|
||
| # Setup some colors to use. These need to work in fairly limited shells, like the Ubuntu Docker container where there are only 8 colors. | ||
| # See if stdout is a terminal | ||
| if [ -t 1 ] && command -v tput > /dev/null; then | ||
| # see if it supports colors | ||
| ncolors=$(tput colors || echo 0) | ||
| if [ -n "$ncolors" ] && [ $ncolors -ge 8 ]; then | ||
| bold="$(tput bold || echo)" | ||
| normal="$(tput sgr0 || echo)" | ||
| black="$(tput setaf 0 || echo)" | ||
| red="$(tput setaf 1 || echo)" | ||
| green="$(tput setaf 2 || echo)" | ||
| yellow="$(tput setaf 3 || echo)" | ||
| blue="$(tput setaf 4 || echo)" | ||
| magenta="$(tput setaf 5 || echo)" | ||
| cyan="$(tput setaf 6 || echo)" | ||
| white="$(tput setaf 7 || echo)" | ||
| fi | ||
| fi | ||
|
|
||
| say_warning() { | ||
| printf "%b\n" "${yellow:-}dotnet_install: Warning: $1${normal:-}" >&3 | ||
| } | ||
|
|
||
| say_err() { | ||
| printf "%b\n" "${red:-}dotnet_install: Error: $1${normal:-}" >&2 | ||
| } | ||
|
|
||
| say() { | ||
| # using stream 3 (defined in the beginning) to not interfere with stdout of functions | ||
| # which may be used as return value | ||
| printf "%b\n" "${cyan:-}dotnet-install:${normal:-} $1" >&3 | ||
| } | ||
|
|
||
| say_verbose() { | ||
| if [ "$verbose" = true ]; then | ||
| say "$1" | ||
| fi | ||
| } | ||
|
|
||
| # This platform list is finite - if the SDK/Runtime has supported Linux distribution-specific assets, | ||
| # then and only then should the Linux distribution appear in this list. | ||
| # Adding a Linux distribution to this list does not imply distribution-specific support. | ||
| get_legacy_os_name_from_platform() { | ||
| eval $invocation | ||
|
|
||
| platform="$1" | ||
| case "$platform" in | ||
| "centos.7") | ||
| echo "centos" | ||
| return 0 | ||
| ;; | ||
| "debian.8") | ||
| echo "debian" | ||
| return 0 | ||
| ;; | ||
| "debian.9") | ||
| echo "debian.9" | ||
| return 0 | ||
| ;; | ||
| "fedora.23") | ||
| echo "fedora.23" | ||
| return 0 | ||
| ;; | ||
| "fedora.24") | ||
| echo "fedora.24" | ||
| return 0 | ||
| ;; | ||
| "fedora.27") | ||
| echo "fedora.27" | ||
| return 0 | ||
| ;; | ||
| "fedora.28") | ||
| echo "fedora.28" | ||
| return 0 | ||
| ;; | ||
| "opensuse.13.2") | ||
| echo "opensuse.13.2" | ||
| return 0 | ||
| ;; | ||
| "opensuse.42.1") | ||
| echo "opensuse.42.1" | ||
| return 0 | ||
| ;; | ||
| "opensuse.42.3") | ||
| echo "opensuse.42.3" | ||
| return 0 | ||
| ;; | ||
| "rhel.7"*) | ||
| echo "rhel" | ||
| return 0 | ||
| ;; | ||
| "ubuntu.14.04") | ||
| echo "ubuntu" | ||
| return 0 | ||
| ;; | ||
| "ubuntu.16.04") | ||
| echo "ubuntu.16.04" | ||
| return 0 | ||
| ;; | ||
| "ubuntu.16.10") | ||
| echo "ubuntu.16.10" | ||
| return 0 | ||
| ;; | ||
| "ubuntu.18.04") | ||
| echo "ubuntu.18.04" | ||
| return 0 | ||
| ;; | ||
| "alpine.3.4.3") | ||
| echo "alpine" | ||
| return 0 | ||
| ;; | ||
| esac | ||
| return 1 | ||
| } | ||
|
|
There was a problem hiding this comment.
This adds the full upstream dotnet-install.sh script (~1.8k lines) at repo root, but the CI workflow already uses actions/setup-dotnet with global.json and there are no references to this script in the repo. Keeping a vendored installer script increases maintenance burden and repo size; consider removing it, or moving it under a dedicated scripts/eng folder with a clear update policy/version pin and documented usage.
Suggested change
| # Copyright (c) .NET Foundation and contributors. All rights reserved. | |
| # Licensed under the MIT license. See LICENSE file in the project root for full license information. | |
| # | |
| # Stop script on NZEC | |
| set -e | |
| # Stop script if unbound variable found (use ${var:-} if intentional) | |
| set -u | |
| # By default cmd1 | cmd2 returns exit code of cmd2 regardless of cmd1 success | |
| # This is causing it to fail | |
| set -o pipefail | |
| # Use in the the functions: eval $invocation | |
| invocation='say_verbose "Calling: ${yellow:-}${FUNCNAME[0]} ${green:-}$*${normal:-}"' | |
| # standard output may be used as a return value in the functions | |
| # we need a way to write text on the screen in the functions so that | |
| # it won't interfere with the return value. | |
| # Exposing stream 3 as a pipe to standard output of the script itself | |
| exec 3>&1 | |
| # Setup some colors to use. These need to work in fairly limited shells, like the Ubuntu Docker container where there are only 8 colors. | |
| # See if stdout is a terminal | |
| if [ -t 1 ] && command -v tput > /dev/null; then | |
| # see if it supports colors | |
| ncolors=$(tput colors || echo 0) | |
| if [ -n "$ncolors" ] && [ $ncolors -ge 8 ]; then | |
| bold="$(tput bold || echo)" | |
| normal="$(tput sgr0 || echo)" | |
| black="$(tput setaf 0 || echo)" | |
| red="$(tput setaf 1 || echo)" | |
| green="$(tput setaf 2 || echo)" | |
| yellow="$(tput setaf 3 || echo)" | |
| blue="$(tput setaf 4 || echo)" | |
| magenta="$(tput setaf 5 || echo)" | |
| cyan="$(tput setaf 6 || echo)" | |
| white="$(tput setaf 7 || echo)" | |
| fi | |
| fi | |
| say_warning() { | |
| printf "%b\n" "${yellow:-}dotnet_install: Warning: $1${normal:-}" >&3 | |
| } | |
| say_err() { | |
| printf "%b\n" "${red:-}dotnet_install: Error: $1${normal:-}" >&2 | |
| } | |
| say() { | |
| # using stream 3 (defined in the beginning) to not interfere with stdout of functions | |
| # which may be used as return value | |
| printf "%b\n" "${cyan:-}dotnet-install:${normal:-} $1" >&3 | |
| } | |
| say_verbose() { | |
| if [ "$verbose" = true ]; then | |
| say "$1" | |
| fi | |
| } | |
| # This platform list is finite - if the SDK/Runtime has supported Linux distribution-specific assets, | |
| # then and only then should the Linux distribution appear in this list. | |
| # Adding a Linux distribution to this list does not imply distribution-specific support. | |
| get_legacy_os_name_from_platform() { | |
| eval $invocation | |
| platform="$1" | |
| case "$platform" in | |
| "centos.7") | |
| echo "centos" | |
| return 0 | |
| ;; | |
| "debian.8") | |
| echo "debian" | |
| return 0 | |
| ;; | |
| "debian.9") | |
| echo "debian.9" | |
| return 0 | |
| ;; | |
| "fedora.23") | |
| echo "fedora.23" | |
| return 0 | |
| ;; | |
| "fedora.24") | |
| echo "fedora.24" | |
| return 0 | |
| ;; | |
| "fedora.27") | |
| echo "fedora.27" | |
| return 0 | |
| ;; | |
| "fedora.28") | |
| echo "fedora.28" | |
| return 0 | |
| ;; | |
| "opensuse.13.2") | |
| echo "opensuse.13.2" | |
| return 0 | |
| ;; | |
| "opensuse.42.1") | |
| echo "opensuse.42.1" | |
| return 0 | |
| ;; | |
| "opensuse.42.3") | |
| echo "opensuse.42.3" | |
| return 0 | |
| ;; | |
| "rhel.7"*) | |
| echo "rhel" | |
| return 0 | |
| ;; | |
| "ubuntu.14.04") | |
| echo "ubuntu" | |
| return 0 | |
| ;; | |
| "ubuntu.16.04") | |
| echo "ubuntu.16.04" | |
| return 0 | |
| ;; | |
| "ubuntu.16.10") | |
| echo "ubuntu.16.10" | |
| return 0 | |
| ;; | |
| "ubuntu.18.04") | |
| echo "ubuntu.18.04" | |
| return 0 | |
| ;; | |
| "alpine.3.4.3") | |
| echo "alpine" | |
| return 0 | |
| ;; | |
| esac | |
| return 1 | |
| } | |
| # | |
| # This repository does not use a vendored copy of the upstream dotnet-install.sh | |
| # script as part of its supported build or CI flow. CI resolves the .NET SDK via | |
| # actions/setup-dotnet and the version pinned in global.json. | |
| # | |
| # If a script-based installation flow is needed in the future, add it under a | |
| # dedicated engineering/scripts location with a documented update policy and | |
| # explicit version pinning instead of restoring a full upstream installer here. | |
| set -eu | |
| printf '%s\n' \ | |
| 'This repository does not support using ./dotnet-install.sh from the repo root.' \ | |
| 'Use the documented toolchain setup for this project (actions/setup-dotnet with global.json).' \ | |
| 'If script-based installation is required, add a maintained wrapper under scripts/eng with documentation.' \ | |
| >&2 | |
| exit 1 |
|
|
||
| STEP 1: LOCATE SOLUTION | ||
| ------------------------ | ||
| ✅ Found: MyBlog.slnx in /home/mpaulosky/Repos/MyBlog.worktrees/copilot-worktree-2026-04-20T15-34-36 |
There was a problem hiding this comment.
This log file includes an absolute local filesystem path with a username (e.g., /home/mpaulosky/...). Committing machine-specific paths can leak personal/workstation info and makes the doc noisy/non-reproducible; please remove this log from the repo (and, if needed, add it to .gitignore) or replace it with a sanitized, path-independent documentation summary.
Suggested change
| ✅ Found: MyBlog.slnx in /home/mpaulosky/Repos/MyBlog.worktrees/copilot-worktree-2026-04-20T15-34-36 | |
| ✅ Found: MyBlog.slnx in repository worktree |
Owner
Author
|
DevOps Review: CI workflow artifact uploads are operationally sound. All three steps correctly use actions/upload-artifact@v7 with if: always() for reliable capture on test success and failure. 30-day retention is cost-appropriate for CI artifacts. Paths correctly target generated test-results directories. Ready for merge from infrastructure perspective. |
… tests (#73) - Add Upload Architecture Test Results step after architecture tests - Add Upload Unit Test Results step after unit tests - Add Upload Integration Test Results step after integration tests - All uploads use actions/upload-artifact@v7 with if: always() and retention-days: 30 - Artifacts named: architecture-test-results, unit-test-results, integration-test-results Closes #73 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mpaulosky
force-pushed
the
squad/73-add-test-artifact-uploads
branch
from
April 20, 2026 17:44
662ca7e to
1a8ab0f
Compare
Contributor
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds artifact upload steps to the CI workflow to capture and display test results from all test suites.
Changes
Upload Architecture Test Resultsstep after Architecture TestsUpload Unit Test Resultsstep after Unit TestsUpload Integration Test Resultsstep after Integration Testsif: always()to capture results even on test failuresArtifacts Configured
Benefits
✅ Test results are now visible in workflow run artifacts
✅ Results accessible directly from PR view for easier review
✅ All test results captured even when tests fail
✅ 30-day retention policy for artifact storage
Closes #73