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.

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

breaking changes run in ci #16170

Merged
merged 29 commits into from
Jan 15, 2021
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
5286730
adding some work, very wip
kristapratico Dec 3, 2020
32ba488
running from tox now
kristapratico Dec 9, 2020
ef0b148
refactored breaking changes checkers into class
kristapratico Dec 10, 2020
cb163ea
major refactoring of checkers - no longer dividing client, now lookin…
kristapratico Dec 10, 2020
4949d43
another major refactor, added more checkers, moved to scripts dir
kristapratico Dec 12, 2020
cf6a594
a few fixes, better names
kristapratico Dec 12, 2020
da54701
changed function report, added new checks
kristapratico Dec 13, 2020
9afa5ab
update to ivars so i'm not passing so many parameters into check funcs
kristapratico Dec 14, 2020
727299f
adding first tests, fixing bugs
kristapratico Dec 15, 2020
17ecb43
adding check for ignore breaking changes, fixes a few bugs
kristapratico Dec 16, 2020
78cc659
added more tests, moved tracker to its own file, fixed ignore, starte…
kristapratico Dec 17, 2020
b91ead0
added checker for removed kwargs
kristapratico Dec 17, 2020
7c093e7
fix table in readme
kristapratico Dec 17, 2020
d9684bf
fixing naming
kristapratico Dec 17, 2020
5f01400
add more docs
kristapratico Dec 17, 2020
654bf25
fix readme formatting
kristapratico Dec 17, 2020
a0cf5a0
rename model to generic class
kristapratico Dec 17, 2020
07bb521
bug fixes, redo tests
kristapratico Dec 19, 2020
21a2415
merge master
kristapratico Jan 4, 2021
7a7ec52
cleanup
kristapratico Jan 13, 2021
7f51081
Merge branch 'master' into detect-breaking-changes
kristapratico Jan 13, 2021
8189eb8
try run in ci
kristapratico Jan 13, 2021
6ba453d
intentionally create a breaking change in form
kristapratico Jan 13, 2021
143a2a7
try removing extra steps
kristapratico Jan 13, 2021
cb0825b
make sure passes with no breaking changes in ci
kristapratico Jan 13, 2021
4ad6b45
remove from breaking changes allowlist and make sure doesn't run
kristapratico Jan 13, 2021
690640a
add form and text to allowlist
kristapratico Jan 14, 2021
df338f5
fix tox.ini config
kristapratico Jan 14, 2021
5189785
revert options
kristapratico Jan 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
7 changes: 7 additions & 0 deletions eng/pipelines/templates/steps/analyze.yml
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,10 @@ steps:
BuildTargetingString: ${{ parameters.BuildTargetingString }}
TestMarkArgument: ${{ parameters.TestMarkArgument }}
AdditionalTestArgs: ${{parameters.AdditionalTestArgs}}

- template: ../steps/run_breaking_changes.yml
parameters:
ServiceDirectory: ${{ parameters.ServiceDirectory }}
BuildTargetingString: ${{ parameters.BuildTargetingString }}
TestMarkArgument: ${{ parameters.TestMarkArgument }}
AdditionalTestArgs: ${{parameters.AdditionalTestArgs}}
19 changes: 19 additions & 0 deletions eng/pipelines/templates/steps/run_breaking_changes.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
parameters:
BuildTargetingString: 'azure-*'
ServiceDirectory: ''
TestMarkArgument: ''
EnvVars: {}

steps:
- task: PythonScript@0
displayName: 'Run Breaking Changes'
inputs:
scriptPath: 'scripts/devops_tasks/setup_execute_tests.py'
arguments: >-
"${{ parameters.BuildTargetingString }}"
--mark_arg="${{ parameters.TestMarkArgument }}"
--service="${{ parameters.ServiceDirectory }}"
--toxenv="breaking"
--disablecov
env: ${{ parameters.EnvVars }}
condition: and(succeededOrFailed(), ne(variables['Skip.BreakingChanges'],'true'))
13 changes: 13 additions & 0 deletions eng/tox/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -266,3 +266,16 @@ deps =
commands =
{envbindir}/python -m pip freeze
{envbindir}/python {toxinidir}/../../../scripts/devops_tasks/test_run_samples.py -t {toxinidir}


[testenv:breaking]
skipsdist = false
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
skipsdist = false
skipsdist = true

skip_install = false
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
skip_install = false
skip_install = true

usedevelop = true
Copy link
Member

@scbedd scbedd Jan 14, 2021

Choose a reason for hiding this comment

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

usedevelop in tox land tells it to install with -e. We don't want to install anything for this tox env, so probably set to false? Honestly it doesn't really matter given that the previous two comments shut off this being used 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.

Thanks Scott! I'm laughing at how I was 0/3 on those config options 😆

Copy link
Member Author

Choose a reason for hiding this comment

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

My bad for not understanding these options, we definitely need to install the current package for this check to work.

https://dev.azure.com/azure-sdk/public/_build/results?buildId=688098&view=logs&j=b70e5e73-bbb6-5567-0939-8415943fadb9&t=efe9ac6d-3fdb-50af-24e1-4319f36eecb4

changedir = {toxinidir}
deps =
{[base]deps}
jsondiff==1.2.0
-e {toxinidir}/../../scripts/breaking_changes_checker
commands =
{envbindir}/python {toxinidir}/../../../scripts/breaking_changes_checker/detect_breaking_changes.py -t {toxinidir}
79 changes: 79 additions & 0 deletions scripts/breaking_changes_checker/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
# Breaking Changes Detector Tool

The breaking changes tool compares the last stable/GA version of the library (if it exists) against the current code
(in master) and reports any breaking changes found.

## How to opt-in to running the tool in CI

Add your package name to the `RUN_BREAKING_CHANGES_PACKAGES` found [here](https://github.com/Azure/azure-sdk-for-python/tree/master/scripts/breaking_changes_checker/breaking_changes_allowlist.py).

## Run locally with tox

**1) Install tox and tox-monorepo:**

`pip install tox tox-monorepo`

**2) Run the `breaking` environment.**

Here we run the breaking changes tool against azure-storage-blob, for example:

`C:\azure-sdk-for-python\sdk\storage\azure-storage-blob>tox -c ../../../eng/tox/tox.ini -e breaking`


## Ignore a reported breaking change

A breaking change reported by this tool may be an acceptable change. If it is an **approved** breaking change (signed off by architecture board)
or a false positive, then you should add the breaking change to the ignore list.

The ignore list is found [here](https://github.com/Azure/azure-sdk-for-python/tree/master/scripts/breaking_changes_checker/breaking_changes_allowlist.py).

To add an ignore, you will need the identifier of the breaking change. This includes the breaking change type,
module name, and optionally class and/or function name, in this order, e.g.

`(breaking-change-type, module-name, class-name, function-name)`

Add this signature as a list of tuples under your package name in the [IGNORE_BREAKING_CHANGES](https://github.com/Azure/azure-sdk-for-python/tree/master/scripts/breaking_changes_checker/breaking_changes_allowlist.py) dictionary.
Note that the names used should be those from the _stable_ package. If I renamed my function from `begin_training` to
`begin_train_model`, I would put `begin_training` as my function name.

See ignore signature skeletons for each type of breaking change [below](#types-of-breaking-changes-detected)

**Examples:**

```python
IGNORE_BREAKING_CHANGES = {
"azure-ai-formrecognizer": [
("RemovedOrRenamedClientMethod", "azure.ai.formrecognizer.aio", "FormTrainingClient", "begin_training"),
("RemovedOrRenamedClass", "azure.ai.formrecognizer", "FormElement"),
],
"azure-storage-queue": [
("RemovedOrRenamedModule", "azure.storage.queue.aio"),
("RemovedOrRenamedModuleLevelFunction", "azure.storage.queue", "generate_queue_sas")
]
}
```


## Types of Breaking Changes Detected

> Note that this does not cover every kind of breaking change possible.
| Breaking Change Type | Explained (changes are relative to the stable/GA library version) | Ignore signature IF an approved breaking change or false positive |
|------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------|------------------------------------------------------------------------------------|
| RemovedOrRenamedModule | An entire module was removed or renamed in the current version. E.g. `aio` module was removed. | ("RemovedOrRenamedModule", "module-name")
| RemovedOrRenamedClient | A client was removed or renamed in the current version. | ("RemovedOrRenamedClient", "module-name", "client-name")
| RemovedOrRenamedClientMethod | A client method was removed or renamed in the current version. | ("RemovedOrRenamedClientMethod", "module-name", "client-name", "function-name")
| RemovedOrRenamedClass | A model or publicly exposed class was removed or renamed in the current version. | ("RemovedOrRenamedClass", "module-name", "class-name")
| RemovedOrRenamedClassMethod | A model or publicly exposed class' method was removed or renamed in the current version. | ("RemovedOrRenamedClassMethod", "module-name", "class-name", "function-name")
| RemovedOrRenamedInstanceAttribute | An instance attribute was removed or renamed in the current version. | ("RemovedOrRenamedInstanceAttribute", "module-name", "class-name")
| RemovedOrRenamedEnumValue | An enum value was removed or renamed in the current version | ("RemovedOrRenamedEnumValue", "module-name", "class-name")
| RemovedOrRenamedModuleLevelFunction | A module level function was removed or renamed in the current version. | ("RemovedOrRenamedModuleLevelFunction", "module-name", "function-name")
| RemovedOrRenamedPositionalParam | A positional parameter on a function was removed or renamed. | ("RemovedOrRenamedPositionalParam", "module-name", "class-name", "function-name")
| AddedPositionalParam | `def my_function(param1) --> def my_function(param1, param2)` | ("AddedPositionalParam", "module-name", "class-name", "function-name")
| RemovedParameterDefaultValue | `def my_function(param=None) --> def my_function(param)` | ("RemovedParameterDefaultValue", "module-name", "class-name", "function-name")
| ChangedParameterDefaultValue | `def my_function(param="yellow") --> def my_function(param="blue")` | ("ChangedParameterDefaultValue", "module-name", "class-name", "function-name")
| ChangedParameterOrdering | `def my_function(a, b, c=None) --> def my_function(b, c=None, a=None)` | ("ChangedParameterOrdering", "module-name", "class-name", "function-name")
| RemovedFunctionKwargs | A function was changed to no longer accept keyword arguments. `def my_func(param, **kwargs) --> def my_func(param)` | ("RemovedFunctionKwargs", "module-name", "class-name", "function-name")
| ChangedParameterKind | `def my_function(a, b, c) --> def my_function(a, b, *, c)` | ("ChangedParameterKind", "module-name", "class-name", "function-name")
| ChangedFunctionKind | `async def my_function(param) -> def my_function(param)` | ("ChangedFunctionKind", "module-name", "class-name", "function-name")

Empty file.
17 changes: 17 additions & 0 deletions scripts/breaking_changes_checker/breaking_changes_allowlist.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#!/usr/bin/env python

# --------------------------------------------------------------------------------------------
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for license information.
# --------------------------------------------------------------------------------------------


RUN_BREAKING_CHANGES_PACKAGES = [
"azure-ai-formrecognizer",
"azure-ai-textanalytics"
]


# See Readme for ignore format

IGNORE_BREAKING_CHANGES = {}
Loading