Skip to content

Conversation

@paulmedynski
Copy link
Contributor

Work in progress.

Copilot AI review requested due to automatic review settings February 2, 2026 14:34
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 refactors SQL Server configuration in Azure DevOps pipelines by centralizing password management and adding diagnostic capabilities for macOS agent setup.

Changes:

  • Removed password parameter passing through template layers, replacing it with direct use of the $(Password) variable from the ADO Library "ADO Test Configuration Properties"
  • Removed temporary password generation/verification steps from the test job template
  • Added documentation comments explaining the Password variable source across all SQL Server configuration step templates
  • Enhanced macOS SQL Server setup with timestamp-based diagnostic output and improved error message clarity
  • Cleaned up trailing whitespace throughout the pipeline files

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml Removed password parameter, replaced all parameter references with $(Password) variable, added documentation comment, cleaned up trailing whitespace
eng/pipelines/common/templates/steps/configure-sql-server-step.yml Removed password parameter definition and parameter passing to child templates
eng/pipelines/common/templates/steps/configure-sql-server-macos-step.yml Removed password parameter, replaced with $(Password) variable, added documentation, added PS4 timestamp prompt for diagnostics, improved error log message
eng/pipelines/common/templates/steps/configure-sql-server-linux-step.yml Removed password parameter, replaced with $(Password) variable, added documentation comment, cleaned up trailing whitespace
eng/pipelines/common/templates/jobs/ci-run-tests-job.yml Removed password generation and verification steps that created a temporary GUID-based password

Copilot AI review requested due to automatic review settings February 2, 2026 15:15
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 7 out of 7 changed files in this pull request and generated 2 comments.

…to arithmetically add extra time to macOS jobs.
@paulmedynski paulmedynski added the Area\Engineering Use this for issues that are targeted for changes in the 'eng' folder or build systems. label Feb 2, 2026
- name: operatingSystem
type: string
default: ''
values:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ensured we're defining this parameter as an enum everywhere since we're doing string comparisons to make decisions based on it.

- ${{ if ne(parameters.prebuildSteps, '') }}:
- ${{ parameters.prebuildSteps }} # extra steps to run before the build like downloading sni and the required configuration

- powershell: |
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We dont' need to generate a GUID and use it as a password. Our Library already has a $(Password) variable meant for SQL Server SA and User passwords.

echo $SQLCMD_ERRORS
echo "Errors will be written to: $SQLCMD_ERRORS"

# Configure the prompt to show the current timestamp so we can see how long each command takes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Diagnostic to see how long each command takes. Azure DevOps refuses to consistently show timestamps.

displayName: Test job timeout (in minutes)
type: number
default: 60
default: 70
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's no way to supply the job timeout based on arithmetic because of the way ci-run-tests-job is designed, so we need to increase the timeout for all test jobs. Fingers crossed an extra 10 minutes eliminates the macOS cancellations.

@codecov
Copy link

codecov bot commented Feb 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.57%. Comparing base (88de2b9) to head (352833b).
⚠️ Report is 3 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (88de2b9) and HEAD (352833b). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (88de2b9) HEAD (352833b)
addons 1 0
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3928      +/-   ##
==========================================
- Coverage   74.93%   67.57%   -7.36%     
==========================================
  Files         269      263       -6     
  Lines       43247    66191   +22944     
==========================================
+ Hits        32407    44731   +12324     
- Misses      10840    21460   +10620     
Flag Coverage Δ
addons ?
netcore 67.64% <ø> (-7.35%) ⬇️
netfx 66.45% <ø> (-7.71%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Labels

Area\Engineering Use this for issues that are targeted for changes in the 'eng' folder or build systems.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants