Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
4 changes: 4 additions & 0 deletions eng/pipelines/common/variables.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ variables:
value: ${{ and(eq(variables['System.TeamProject'], 'public'), ne(variables['Build.Reason'], 'PullRequest')) }}
- name: isNotFullMatrix
value: ${{ and(eq(variables['System.TeamProject'], 'public'), eq(variables['Build.Reason'], 'PullRequest')) }}
- name: isNotManualAndIsPR
value: ${{ and(not(in(variables['Build.DefinitionName'], 'manual-runtime-staging', 'manual-runtime')), eq(variables['System.TeamProject'], 'public'), eq(variables['Build.Reason'], 'PullRequest')) }}
- name: isManualOrIsNotPR
Copy link
Member

Choose a reason for hiding this comment

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

I'd call the variable isReducedFullMatrix since we're essentially running a bit less than the normal fullmatrix run, I don't think we need to tie this into PR vs. rolling vs. manual.

Copy link
Member

@akoeplinger akoeplinger Nov 11, 2021

Choose a reason for hiding this comment

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

alternatively, I wonder if we should instead just set isFullMatrix for the manual pipeline and only update the few places where we really really don't want to run on PRs even in the manual pipeline.

Copy link
Member Author

Choose a reason for hiding this comment

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

alternatively, I wonder if we should instead just set isFullMatrix for the manual pipeline and only update the few places where we really really don't want to run on PRs even in the manual pipeline.

For runtime-staging that might be ok, but I think it would be harder to work around in runtime. That's why I thought Santi's suggestion for another variable was a good one.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd call the variable isReducedFullMatrix since we're essentially running a bit less than the normal fullmatrix run, I don't think we need to tie this into PR vs. rolling vs. manual.

I thought Ankit's suggestion was easier to work with when modifying the yml. Honestly, I don't care what we name it.

@radical @safern, thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

When I quickly looked it didn't seem used in a lot of relevant places but ok :)

That actually raises another question, or maybe I'm misunderstanding the conditions: we're running all of the legs in the manual pipeline, even those that already run as part of the normal pipeline right? that seems suboptimal and a waste of resources.

Copy link
Member

Choose a reason for hiding this comment

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

My biggest concern with using isFullMatrix is for runtime.yml because that runs as a rolling build which has a LOOT of tests and also a lot of those would already be covered by the default run we get on PRs, so that's why I suggested a new variable.

Copy link
Member Author

Choose a reason for hiding this comment

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

@akoeplinger @radical @safern Are the isNotManualAndIsPR and isManualOrIsNotPR variables ok or do we want a different name? My vote is to go with them, but I'd like your opinions before moving forward.

Copy link
Member

Choose a reason for hiding this comment

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

I would prefer sticking with them, but would be fine with whatever @safern, and @akoeplinger agree upon.

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with sticking with them too.

Copy link
Member

Choose a reason for hiding this comment

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

Fine with sticking with those names as well.

value: ${{ or(in(variables['Build.DefinitionName'], 'manual-runtime-staging', 'manual-runtime'), and(eq(variables['System.TeamProject'], 'public'), ne(variables['Build.Reason'], 'PullRequest'))) }}

# We only run evaluate paths on runtime, runtime-staging and runtime-community pipelines on PRs
# keep in sync with /eng/pipelines/common/xplat-setup.yml
Expand Down
2 changes: 1 addition & 1 deletion eng/pipelines/common/xplat-setup.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ jobs:
value: $(buildConfigUpper)

- name: _runSmokeTestsOnlyArg
value: '/p:RunSmokeTestsOnly=$(isNotFullMatrix)'
value: '/p:RunSmokeTestsOnly=$(isNotManualAndIsPR)'
- ${{ if or(eq(parameters.osGroup, 'windows'), eq(parameters.hostedOs, 'windows')) }}:
- name: archiveExtension
value: '.zip'
Expand Down
37 changes: 25 additions & 12 deletions eng/pipelines/runtime-staging.yml
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ jobs:
eq(dependencies.evaluate_paths.outputs['SetPathVars_libraries.containsChange'], true),
eq(dependencies.evaluate_paths.outputs['SetPathVars_mono.containsChange'], true),
eq(dependencies.evaluate_paths.outputs['SetPathVars_installer.containsChange'], true),
eq(variables['isManualOrIsNotPR'], true),
eq(variables['isFullMatrix'], true))
# extra steps, run tests
extraStepsTemplate: /eng/pipelines/libraries/helix.yml
Expand Down Expand Up @@ -137,6 +138,7 @@ jobs:
eq(dependencies.evaluate_paths.outputs['SetPathVars_libraries.containsChange'], true),
eq(dependencies.evaluate_paths.outputs['SetPathVars_mono.containsChange'], true),
eq(dependencies.evaluate_paths.outputs['SetPathVars_installer.containsChange'], true),
eq(variables['isManualOrIsNotPR'], true),
eq(variables['isFullMatrix'], true))
# extra steps, run tests
extraStepsTemplate: /eng/pipelines/libraries/helix.yml
Expand Down Expand Up @@ -179,6 +181,7 @@ jobs:
eq(dependencies.evaluate_paths.outputs['SetPathVars_libraries.containsChange'], true),
eq(dependencies.evaluate_paths.outputs['SetPathVars_mono.containsChange'], true),
eq(dependencies.evaluate_paths.outputs['SetPathVars_installer.containsChange'], true),
eq(variables['isManualOrIsNotPR'], true),
eq(variables['isFullMatrix'], true))
# extra steps, run tests
extraStepsTemplate: /eng/pipelines/libraries/helix.yml
Expand All @@ -190,6 +193,7 @@ jobs:
or(
eq(variables['librariesContainsChange'], true),
eq(variables['monoContainsChange'], true),
eq(variables['isManualOrIsNotPR'], true),
eq(variables['isFullMatrix'], true))

#
Expand Down Expand Up @@ -220,6 +224,7 @@ jobs:
eq(dependencies.evaluate_paths.outputs['SetPathVars_libraries.containsChange'], true),
eq(dependencies.evaluate_paths.outputs['SetPathVars_mono.containsChange'], true),
eq(dependencies.evaluate_paths.outputs['SetPathVars_installer.containsChange'], true),
eq(variables['isManualOrIsNotPR'], true),
eq(variables['isFullMatrix'], true))
# extra steps, run tests
extraStepsTemplate: /eng/pipelines/libraries/helix.yml
Expand All @@ -230,6 +235,7 @@ jobs:
or(
eq(variables['librariesContainsChange'], true),
eq(variables['monoContainsChange'], true),
eq(variables['isManualOrIsNotPR'], true),
eq(variables['isFullMatrix'], true))

- template: /eng/pipelines/common/platform-matrix.yml
Expand Down Expand Up @@ -257,19 +263,19 @@ jobs:
eq(dependencies.evaluate_paths.outputs['SetPathVars_libraries.containsChange'], true),
eq(dependencies.evaluate_paths.outputs['SetPathVars_mono.containsChange'], true),
eq(dependencies.evaluate_paths.outputs['SetPathVars_installer.containsChange'], true),
eq(variables['isManualOrIsNotPR'], true),
eq(variables['isFullMatrix'], true))
# extra steps, run tests
extraStepsTemplate: /eng/pipelines/libraries/helix.yml
extraStepsParameters:
creator: dotnet-bot
testRunNamePrefixSuffix: Mono_$(_BuildConfig)
condition: >-
or(
eq(variables['librariesContainsChange'], true),
eq(variables['monoContainsChange'], true),
eq(variables['isManualOrIsNotPR'], true),
eq(variables['isFullMatrix'], true))
# Device tests are rolling build only
${{ if eq(variables['isFullMatrix'], true) }}:
# extra steps, run tests
extraStepsTemplate: /eng/pipelines/libraries/helix.yml
extraStepsParameters:
creator: dotnet-bot
testRunNamePrefixSuffix: Mono_$(_BuildConfig)
condition: >-
or(
eq(variables['librariesContainsChange'], true),
eq(variables['monoContainsChange'], true),
eq(variables['isFullMatrix'], true))

#
# Build the whole product using Mono and run libraries tests
Expand Down Expand Up @@ -298,6 +304,7 @@ jobs:
eq(dependencies.evaluate_paths.outputs['SetPathVars_libraries.containsChange'], true),
eq(dependencies.evaluate_paths.outputs['SetPathVars_mono.containsChange'], true),
eq(dependencies.evaluate_paths.outputs['SetPathVars_installer.containsChange'], true),
eq(variables['isManualOrIsNotPR'], true),
eq(variables['isFullMatrix'], true))
# extra steps, run tests
extraStepsTemplate: /eng/pipelines/libraries/helix.yml
Expand All @@ -308,6 +315,7 @@ jobs:
or(
eq(variables['librariesContainsChange'], true),
eq(variables['monoContainsChange'], true),
eq(variables['isManualOrIsNotPR'], true),
eq(variables['isFullMatrix'], true))

#
Expand Down Expand Up @@ -342,6 +350,7 @@ jobs:
or(
eq(dependencies.evaluate_paths.outputs['SetPathVars_runtimetests.containsChange'], true),
eq(dependencies.evaluate_paths.outputs['SetPathVars_mono.containsChange'], true),
eq(variables['isManualOrIsNotPR'], true),
eq(variables['isFullMatrix'], true))
# NOTE: Per PR test execution is not recommended for runtime tests
${{ if eq(variables['isFullMatrix'], true) }}:
Expand Down Expand Up @@ -382,6 +391,7 @@ jobs:
or(
eq(dependencies.evaluate_paths.outputs['SetPathVars_runtimetests.containsChange'], true),
eq(dependencies.evaluate_paths.outputs['SetPathVars_mono.containsChange'], true),
eq(variables['isManualOrIsNotPR'], true),
eq(variables['isFullMatrix'], true))
${{ if eq(variables['isFullMatrix'], true) }}:
# extra steps, run tests
Expand Down Expand Up @@ -421,6 +431,7 @@ jobs:
or(
eq(dependencies.evaluate_paths.outputs['SetPathVars_runtimetests.containsChange'], true),
eq(dependencies.evaluate_paths.outputs['SetPathVars_mono.containsChange'], true),
eq(variables['isManualOrIsNotPR'], true),
eq(variables['isFullMatrix'], true))
# don't run tests on PRs until we can get significantly more devices
# Turn off the testing for now, until https://github.com/dotnet/xharness/issues/663 gets resolved
Expand Down Expand Up @@ -458,6 +469,7 @@ jobs:
eq(dependencies.evaluate_paths.outputs['SetPathVars_libraries.containsChange'], true),
eq(dependencies.evaluate_paths.outputs['SetPathVars_mono.containsChange'], true),
eq(dependencies.evaluate_paths.outputs['SetPathVars_installer.containsChange'], true),
eq(variables['isManualOrIsNotPR'], true),
eq(variables['isFullMatrix'], true))
${{ if eq(variables['isFullMatrix'], true) }}:
# extra steps, run tests
Expand All @@ -473,6 +485,7 @@ jobs:
or(
eq(variables['librariesContainsChange'], true),
eq(variables['monoContainsChange'], true),
eq(variables['isManualOrIsNotPR'], true),
eq(variables['isFullMatrix'], true))

#
Expand Down