Update test templates to check weekly runs based on run start time - #48391
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 9 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
/azp run python - core - tests |
|
Azure Pipelines: Successfully started running 1 pipeline(s). 9 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
/azp run python - template - tests |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
Updates weekly analysis scheduling and Python CI context detection.
Changes:
- Adds weekend schedule checks and gates analysis stages.
- Adds configurable analysis-stage dependencies and conditions.
- Replaces pipeline-name detection with an explicit environment marker.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
eng/tools/azure-sdk-tools/ci_tools/variables.py |
Detects weekly analysis via an explicit variable. |
eng/pipelines/templates/stages/python-analyze-weekly.yml |
Adds gating parameters and the analysis marker. |
eng/pipelines/templates/stages/python-analyze-weekly-standalone.yml |
Adds scheduled weekend gating. |
eng/pipelines/templates/stages/archetype-sdk-tests.yml |
Adds schedule gating for weekly analysis and modifies cloud selection. |
| - ${{ each package in coalesce(parameters.Packages, split(parameters.BuildTargetingString, '|')) }}: | ||
| - ${{ each cloud in parameters.CloudConfig }}: | ||
| - ${{ if or(contains(parameters.Clouds, cloud.key), and(contains(variables['Build.DefinitionName'], 'tests-weekly'), contains(parameters.SupportedClouds, cloud.key))) }}: | ||
| - ${{ if contains(parameters.Clouds, cloud.key) }}: |
There was a problem hiding this comment.
12 services set it, but 9 set it to Public (same as the default) — those are unaffected. Only 3 set a non-Public value:
-- | -- | -- | --
Service | Clouds | CloudConfig | Affected?
personalizer | Canary | default (includes Public) | Yes (Can be ignored coz the last run was 2022)
formrecognizer | Prod | custom — only Prod | No
agrifood | Public_FarmBeats | custom — only Public_Farmbeats | No
Co-authored-by: raych1 <20296335+raych1@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
eng/pipelines/templates/stages/archetype-sdk-tests.yml:134
- This also removes the existing weekly expansion to
SupportedClouds, although the PR describes changing only the analyze-stage gate. For example,sdk/personalizer/tests.yml:8setsClouds: Canaryand relies on the defaultSupportedClouds: Public; its weekly pipeline previously ran both clouds but will now never create the Public stage. Preserve the supported-cloud stages with a weekendCheckSchedulegate, or document and update callers if dropping weekly multi-cloud coverage is intentional.
- ${{ if contains(parameters.Clouds, cloud.key) }}:
eng/pipelines/templates/stages/archetype-sdk-tests.yml:126
- This computes the weekend from when the
CheckScheduleagent happens to execute, not from the pipeline start as the comment and PR require. A queued or delayed run that crosses midnight at the weekend boundary can therefore incorrectly enable or skip weekly analysis.
$now = [System.TimeZoneInfo]::ConvertTimeFromUtc([datetime]::UtcNow, $tz)
Co-authored-by: raych1 <20296335+raych1@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (2)
eng/pipelines/templates/stages/python-analyze-weekly-standalone.yml:33
Build.QueuedTimeis not a predefined Azure Pipelines variable, so it remains the literal$(Build.QueuedTime)here and thedatetimeoffsetcast fails. That makesCheckSchedulefail and prevents every standalone weekly analysis stage from running. Define the samepipeline.startTimeruntime variable used byarchetype-sdk-tests.ymland parse that value instead.
$runStart = ([datetimeoffset]'$(Build.QueuedTime)').UtcDateTime
eng/pipelines/templates/stages/archetype-sdk-tests.yml:137
- This removes the existing weekly expansion to
SupportedCloudseven though the PR only changes analysis-stage gating, and leaves that parameter unused. For example,sdk/personalizer/tests.yml:8setsClouds: Canarywhile retaining the defaultSupportedClouds: Public; its weekly pipeline previously tested both clouds but will now test only Canary. Preserve the existing cloud-selection condition independently of the new analyze gate.
- ${{ if contains(parameters.Clouds, cloud.key) }}:
Daniel Jurek (danieljurek)
left a comment
There was a problem hiding this comment.
LGTM, not blocking but I think this can be refactored into a single stage template.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
eng/pipelines/templates/stages/archetype-sdk-tests.yml:126
- The current diff reintroduces the boundary bug:
[datetime]::UtcNowis the agent step's execution time, not the pipeline start time promised by this gate. A scheduled run queued before a weekend boundary can therefore be classified using the wrong day. Derive the timezone conversion from an immutable pipeline start/queue timestamp instead.
$now = [System.TimeZoneInfo]::ConvertTimeFromUtc([datetime]::UtcNow, $tz)
|
/azp run python - formrecognizer - tests |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
/azp run python - agrifood - tests |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
As discussed offline I think it would be better to use the built-in For matrices that we include conditionally we can start by keeping those weekly pipelines active as legacy while we explore dynamic solutions. |
| $tz = [System.TimeZoneInfo]::FindSystemTimeZoneById('${{ parameters.TimeZoneId }}') | ||
| $local = [System.TimeZoneInfo]::ConvertTimeFromUtc($startUtc, $tz) | ||
| $weekend = $local.DayOfWeek -in 'Saturday','Sunday' | ||
| Write-Host "PipelineStartTime raw='$raw' -> UTC='$startUtc' -> $($tz.Id)='$local' -> IsWeekend=$weekend" |
There was a problem hiding this comment.
|
/azp run python - template - tests |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
/azp run python - template - tests |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
eng/pipelines/templates/stages/archetype-sdk-tests.yml:178
- The PR description says weekly analysis is gated on the
CheckScheduleoutput, but this condition recomputes the decision directly fromBuild.Reasonandpipeline.startTime; noCheckScheduleoutput is referenced anywhere in these changes. Either restore the documented output-based gate or update the PR description so the stated behavior matches the implementation.
Condition: and(succeeded(), in(format('{0:ddd}', pipeline.startTime), 'Sat', 'Sun'))
Summary
Testing