Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Create runtime staging clone to manually kick off full test runs #61443

Merged
merged 14 commits into from
Nov 15, 2021
Merged

Create runtime staging clone to manually kick off full test runs #61443

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
a519323
Create runtime staging clone to manually kick off full test runs
steveisok Nov 11, 2021
725c27f
Remove separate pipeline
Nov 11, 2021
6195dd4
Tweak variables to support full matrix on manual runs
Nov 11, 2021
4f19def
Update eng/pipelines/common/variables.yml
steveisok Nov 11, 2021
d5c6207
Add a separate isSmokeTestBuild variable and trickle it into runtime-…
Nov 11, 2021
3ebaa8c
Merge remote-tracking branch 'upstream/stpfiste/manual-runtime-stagin…
Nov 11, 2021
bd8e5aa
Merge remote-tracking branch 'upstream/main' into stpfiste/manual-run…
Nov 11, 2021
3eaff34
Flip conditions
Nov 11, 2021
ec7f252
Naming feedback
Nov 11, 2021
d19ca34
Flipped name of manual pipeline
Nov 11, 2021
44e7023
Prevent anything being scheduled on the manual pipelines
Nov 11, 2021
b497b31
Use template for runtime-staging so we can extract out the schedule a…
akoeplinger Nov 12, 2021
42caf7c
Exclude DllImportantGenerator Unit tests from mobile
Nov 13, 2021
1390cc8
Make sure a few more configurations run manually
Nov 15, 2021
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
View file
Open in desktop
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'], 'runtime-staging-manual', 'runtime-manual')), 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 •
edited
Loading

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'], 'runtime-staging-manual', 'runtime-manual'), 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
View file
Open in desktop
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
4 changes: 4 additions & 0 deletions eng/pipelines/runtime-staging-manual.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
trigger: none

extends:
template: runtime-staging-template.yml
Copy link
Member

Choose a reason for hiding this comment

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

now that we have a template, maybe we can use parameters to turn on/off legs rather than checking based on pipeline name?

Copy link
Member

Choose a reason for hiding this comment

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

Like platform!

Copy link
Member

Choose a reason for hiding this comment

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

that works for runtime-staging but we still have runtime.yml, so I'd need to move that to template as well which I'm not sure is desired.

Copy link
Member

Choose a reason for hiding this comment

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

That one also has a schedule trigger, so it will limit us as well and we will need the template anyway?

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 think this is a good first step. I think we can get more custom in a follow up PR.

Loading