Skip to content

TestProxy with a switch to start #18466

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

Merged
HarshaNalluru merged 10 commits into from
Nov 10, 2021
Merged

TestProxy with a switch to start #18466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c5a466f
TestProxy
HarshaNalluru Nov 1, 2021
81eca06
TestProxy: not-specified
HarshaNalluru Nov 1, 2021
c962930
Update eng/pipelines/templates/steps/test.yml
HarshaNalluru Nov 1, 2021
7688ff5
feedback
HarshaNalluru Nov 1, 2021
766edb7
Merge branch 'harshan/issue/unleash-test-proxy' of https://github.com…
HarshaNalluru Nov 1, 2021
3936a02
TestProxy: ${{ parameters.TestProxy }}
HarshaNalluru Nov 1, 2021
f4306ab
- name: TestProxy
HarshaNalluru Nov 1, 2021
94b63df
Update eng/pipelines/templates/stages/archetype-sdk-client.yml
HarshaNalluru Nov 1, 2021
2c4deff
Update eng/pipelines/templates/steps/test.yml
HarshaNalluru Nov 9, 2021
fde3d97
Update eng/pipelines/templates/steps/test.yml
HarshaNalluru Nov 9, 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/templates/jobs/ci.tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ parameters:
- name: ServiceDirectory
type: string
default: not-specified
- name: TestProxy
type: boolean
default: false
- name: Artifacts
type: object
default: []
Expand Down Expand Up @@ -52,3 +55,4 @@ jobs:
parameters:
Artifacts: ${{ parameters.Artifacts }}
ServiceDirectory: ${{ parameters.ServiceDirectory }}
TestProxy: ${{ parameters.TestProxy }}
4 changes: 4 additions & 0 deletions eng/pipelines/templates/jobs/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ parameters:
- name: TestPipeline
type: boolean
default: false
- name: TestProxy
type: boolean
default: false
- name: ServiceDirectory
type: string
default: not-specified
Expand Down Expand Up @@ -76,3 +79,4 @@ jobs:
AdditionalParameters:
ServiceDirectory: ${{ parameters.ServiceDirectory }}
Artifacts: ${{ parameters.Artifacts }}
TestProxy: ${{ parameters.TestProxy }}
7 changes: 6 additions & 1 deletion eng/pipelines/templates/stages/archetype-sdk-client.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ parameters:
- name: ServiceDirectory
type: string
default: not-specified
- name: TestProxy
Copy link
Copy Markdown
Member

@scbedd scbedd Nov 1, 2021

Choose a reason for hiding this comment

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

down below we call into ci.yml. We need to add additional parameter so the call looks like this:

      - template: /eng/pipelines/templates/jobs/ci.yml
        parameters:
          ServiceDirectory: ${{ parameters.ServiceDirectory }}
          TestProxy: ${{ parameterse.TestProxy }}
          Artifacts: ${{ parameters.Artifacts }}

Copy link
Copy Markdown
Member

@scbedd scbedd Nov 1, 2021

Choose a reason for hiding this comment

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

Parameters are compile-time constants. So technically it'll work if we miss adding it to each level of templates, but it'll be super confusing to debug. Better to pass all the way through.

Copy link
Copy Markdown
Member

@scbedd scbedd Nov 1, 2021

Choose a reason for hiding this comment

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

IN jobs/ci.yml, we need to add the parameter (and then pass it to ci.tests.yml -> tests.yml)

Copy link
Copy Markdown
Contributor Author

@HarshaNalluru HarshaNalluru Nov 1, 2021

Choose a reason for hiding this comment

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

Does it look good now?

Copy link
Copy Markdown
Member

@scbedd scbedd Nov 1, 2021

Choose a reason for hiding this comment

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

Yes! At least, I think so :)

Copy link
Copy Markdown
Member

@praveenkuttappan praveenkuttappan Nov 1, 2021

Choose a reason for hiding this comment

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

you are not passing this parameter ci.yml. We need to pass this parameter down to test.yml template. Here ci.yml

and https://github.com/Azure/azure-sdk-for-js/blob/main/eng/pipelines/templates/jobs/ci.yml#L78

and https://github.com/Azure/azure-sdk-for-js/blob/main/eng/pipelines/templates/jobs/ci.tests.yml#L54

Copy link
Copy Markdown
Contributor Author

@HarshaNalluru HarshaNalluru Nov 1, 2021

Choose a reason for hiding this comment

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

Does it look good now?

Copy link
Copy Markdown
Member

@scbedd scbedd Nov 1, 2021

Choose a reason for hiding this comment

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

yeah. seems reasonable. now let's see what breaks 👍

type: boolean
default: false
- name: IncludeRelease
type: boolean
default: true
Expand Down Expand Up @@ -42,7 +45,8 @@ stages:
jobs:
- template: /eng/pipelines/templates/jobs/ci.yml
parameters:
ServiceDirectory: ${{ parameters.ServiceDirectory }}
ServiceDirectory: ${{ parameters.ServiceDirectory }}
TestProxy: ${{ parameters.TestProxy }}
Artifacts: ${{ parameters.Artifacts }}
${{ if eq(parameters.ServiceDirectory, 'template') }}:
TestPipeline: true
Expand All @@ -66,6 +70,7 @@ stages:
parameters:
DependsOn: Build
ServiceDirectory: ${{ parameters.ServiceDirectory }}
TestProxy: ${{ parameters.TestProxy }}
Artifacts: ${{ parameters.Artifacts }}
${{ if eq(parameters.ServiceDirectory, 'template') }}:
TestPipeline: true
Expand Down
4 changes: 4 additions & 0 deletions eng/pipelines/templates/steps/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
parameters:
Artifacts: []
ServiceDirectory: not-specified
TestProxy: false

steps:
- template: /eng/common/pipelines/templates/steps/verify-agent-os.yml
Expand All @@ -25,6 +26,9 @@ steps:

- template: ../steps/use-node-test-version.yml

- ${{ if eq(parameters.TestProxy, true) }}:
- template: /eng/common/testproxy/test-proxy-tool.yml

# Option "-p max" ensures parallelism is set to the number of cores on all platforms, which improves build times.
# The default on Windows is "cores - 1" (microsoft/rushstack#436).
- script: |
Expand Down
1 change: 1 addition & 0 deletions sdk/test-utils/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ extends:
parameters:
ServiceDirectory: test-utils
IncludeRelease: false
TestProxy: true
Artifacts:
- name: azure-tools-test-recorder
safeName: azuretoolstestrecorder