Skip to content

Commit 69f0c22

Browse files
authored
Replace compile-time validateFormatting parameter (Azure#38212)
* transition validate_formatting.py to run_black.py called from within the tox env * update filter_tox_env_string to handle check defaults that are nonTrue * default black to opt-in, not opt-out
1 parent 947ad7c commit 69f0c22

File tree

34 files changed

+118
-152
lines changed

34 files changed

+118
-152
lines changed

doc/eng_sys_checks.md

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,8 @@ This is the most useful skip, but the following skip variables are also supporte
112112
- Omit checking that a package's dependencies are on PyPI before releasing.
113113
- `Skip.KeywordCheck`
114114
- Omit checking that a package's keywords are correctly formulated before releasing.
115+
- `Skip.Black`
116+
- Omit checking `black` in the `analyze` job.
115117

116118
## The pyproject.toml
117119

@@ -172,7 +174,7 @@ You can enable test logging in a pipeline by setting the queue time variable `PY
172174

173175
`PYTEST_LOG_LEVEL=INFO`
174176

175-
This also works locally with tox by setting the `PYTEST_LOG_LEVEL` environment variable.
177+
This also works locally with tox by setting the `PYTEST_LOG_LEVEL` environment variable.
176178

177179
Note that if you want DEBUG level logging with sensitive information unredacted in the test logs, then you still must pass `logging_enable=True` into the client(s) being used in tests.
178180

@@ -237,17 +239,17 @@ fail if docstring are invalid, helping to ensure the resulting documentation wil
237239

238240
#### Opt-in to formatting validation
239241

240-
Make the following change to your projects `ci.yml`:
242+
Ensure that `black = true` is present within your `pyproject.toml`:
241243

242244
```yml
243-
extends:
244-
template: ../../eng/pipelines/templates/stages/archetype-sdk-client.yml
245-
parameters:
246-
...
247-
ValidateFormatting: true
248-
...
245+
[tool.azure-sdk-build]
246+
...other checks enabled/disabled
247+
black = true
248+
...other checks enabled/disabled
249249
```
250250

251+
to opt into the black invocation.
252+
251253
#### Running locally
252254

253255
1. Go to package root directory.

eng/pipelines/templates/jobs/ci.yml

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,6 @@ parameters:
4949
- name: VerifyAutorest
5050
type: boolean
5151
default: false
52-
- name: ValidateFormatting
53-
type: boolean
54-
default: false
5552
- name: UnsupportedToxEnvironments
5653
type: string
5754
default: ''
@@ -229,7 +226,6 @@ jobs:
229226
TestPipeline: ${{ parameters.TestPipeline }}
230227
Artifacts: ${{ parameters.Artifacts }}
231228
VerifyAutorest: ${{ parameters.VerifyAutorest }}
232-
ValidateFormatting: ${{ parameters.ValidateFormatting }}
233229
GenerateApiReviewForManualOnly: ${{ parameters.GenerateApiReviewForManualOnly }}
234230

235231
- template: /eng/common/pipelines/templates/jobs/generate-job-matrix.yml

eng/pipelines/templates/stages/archetype-sdk-client.yml

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,6 @@ parameters:
6363
- name: VerifyAutorest
6464
type: boolean
6565
default: false
66-
- name: ValidateFormatting
67-
type: boolean
68-
default: false
6966
- name: TestProxy
7067
type: boolean
7168
default: true
@@ -107,7 +104,6 @@ extends:
107104
MatrixFilters: ${{ parameters.MatrixFilters }}
108105
MatrixReplace: ${{ parameters.MatrixReplace }}
109106
VerifyAutorest: ${{ parameters.VerifyAutorest }}
110-
ValidateFormatting: ${{ parameters.ValidateFormatting }}
111107
TestProxy: ${{ parameters.TestProxy }}
112108
GenerateApiReviewForManualOnly: ${{ parameters.GenerateApiReviewForManualOnly }}
113109

eng/pipelines/templates/steps/analyze.yml

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ parameters:
55
Artifacts: []
66
TestPipeline: false
77
VerifyAutorest: false
8-
ValidateFormatting: false
98
GenerateApiReviewForManualOnly: false
109

1110
# Please use `$(TargetingString)` to refer to the python packages glob string. This variable is set from resolve-package-targeting.yml.
@@ -110,11 +109,9 @@ steps:
110109
ServiceDirectory: ${{ parameters.ServiceDirectory }}
111110
AdditionalTestArgs: ${{ parameters.AdditionalTestArgs }}
112111

113-
- ${{ if parameters.ValidateFormatting }}:
114-
- template: run_black.yml
115-
parameters:
116-
ServiceDirectory: ${{ parameters.ServiceDirectory }}
117-
ValidateFormatting: ${{ parameters.ValidateFormatting }}
112+
- template: run_black.yml
113+
parameters:
114+
ServiceDirectory: ${{ parameters.ServiceDirectory }}
118115

119116
- task: PythonScript@0
120117
displayName: 'Run Keyword Validation Check'

eng/pipelines/templates/steps/run_black.yml

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
parameters:
22
ServiceDirectory: ''
3-
ValidateFormatting: false
43
EnvVars: {}
54
AdditionalTestArgs: ''
65

@@ -19,10 +18,13 @@ steps:
1918
- task: PythonScript@0
2019
displayName: 'Run Black'
2120
inputs:
22-
scriptPath: 'scripts/devops_tasks/validate_formatting.py'
21+
scriptPath: 'scripts/devops_tasks/dispatch_tox.py'
2322
arguments: >-
2423
"$(TargetingString)"
25-
--service_directory="${{ parameters.ServiceDirectory }}"
26-
--validate="${{ parameters.ValidateFormatting }}"
24+
--mark_arg="${{ parameters.TestMarkArgument }}"
25+
--service="${{ parameters.ServiceDirectory }}"
26+
--toxenv="black"
27+
--filter-type="Omit_management"
28+
${{ parameters.AdditionalTestArgs }}
2729
env: ${{ parameters.EnvVars }}
28-
condition: and(succeededOrFailed(), ne(variables['Skip.Pylint'],'true'))
30+
condition: and(succeededOrFailed(), ne(variables['Skip.Black'],'true'))

eng/tox/run_black.py

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
#!/usr/bin/env python
2+
3+
# --------------------------------------------------------------------------------------------
4+
# Copyright (c) Microsoft Corporation. All rights reserved.
5+
# Licensed under the MIT License. See License.txt in the project root for license information.
6+
# --------------------------------------------------------------------------------------------
7+
8+
# This script is used to execute pylint within a tox environment. Depending on which package is being executed against,
9+
# a failure may be suppressed.
10+
11+
import subprocess
12+
import argparse
13+
import os
14+
import logging
15+
import sys
16+
17+
from ci_tools.environment_exclusions import is_check_enabled
18+
from ci_tools.parsing import ParsedSetup
19+
from ci_tools.variables import in_ci
20+
21+
logging.getLogger().setLevel(logging.INFO)
22+
23+
root_dir = os.path.abspath(os.path.join(os.path.abspath(__file__), "..", "..", ".."))
24+
25+
if __name__ == "__main__":
26+
parser = argparse.ArgumentParser(
27+
description="Run black against target folder. Uses the black config in eng/black-pyproject.toml."
28+
)
29+
30+
parser.add_argument(
31+
"-t",
32+
"--target",
33+
dest="target_package",
34+
help="The target package directory on disk.",
35+
required=True,
36+
)
37+
38+
args = parser.parse_args()
39+
40+
pkg_dir = os.path.abspath(args.target_package)
41+
pkg_details = ParsedSetup.from_path(pkg_dir)
42+
configFileLocation = os.path.join(root_dir, "eng/black-pyproject.toml")
43+
44+
if in_ci():
45+
if not is_check_enabled(args.target_package, "black"):
46+
logging.info(
47+
f"Package {pkg_details.name} opts-out of black check."
48+
)
49+
exit(0)
50+
51+
try:
52+
run_result = subprocess.run(
53+
[
54+
sys.executable,
55+
"-m",
56+
"black",
57+
f"--config={configFileLocation}",
58+
args.target_package,
59+
],
60+
stdout=subprocess.PIPE,
61+
stderr=subprocess.PIPE
62+
)
63+
64+
if run_result.stderr and "reformatted" in run_result.stderr.decode("utf-8"):
65+
if in_ci():
66+
logging.info(f"The package {pkg_details.name} needs reformat. Run the `black` tox env locally to reformat.")
67+
exit(1)
68+
else:
69+
logging.info(f"The package {pkg_details.name} was reformatted.")
70+
71+
except subprocess.CalledProcessError as e:
72+
logging.error(
73+
f"Unable to invoke black for {pkg_details.name}. Ran into exception {e}."
74+
)
75+

eng/tox/tox.ini

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -531,9 +531,9 @@ description=Runs the code formatter black
531531
skip_install=true
532532
deps=
533533
black==24.4.0
534+
-rdev_requirements.txt
534535
commands=
535-
black --config {repository_root}/eng/black-pyproject.toml {posargs}
536-
536+
python {repository_root}/eng/tox/run_black.py -t {tox_root}
537537

538538
[testenv:generate]
539539
description=Regenerate the code

scripts/devops_tasks/validate_formatting.py

Lines changed: 0 additions & 107 deletions
This file was deleted.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
[tool.azure-sdk-build]
22
pyright = false
3+
black = true
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
[tool.azure-sdk-build]
22
pyright = false
3+
black = true

0 commit comments

Comments
 (0)