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

Separate tox.ini into multiple files #2508

Draft
wants to merge 27 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion .codespellrc
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
[codespell]
# skipping auto generated folders
skip = ./.tox,./.mypy_cache,./docs/_build,./target,*/LICENSE,./venv
skip = ./.tox,./.mypy_cache,./docs/_build,./target,*/LICENSE,./venv,*-requirements*.txt
ignore-words-list = ot
64 changes: 64 additions & 0 deletions .github/workflows/get_tox_ini_directory_python_versions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
from json import dumps
from configparser import ConfigParser
from os.path import join
from os import walk
from pathlib import Path
from re import compile as re_compile


tox_ini_paths = []

env_regex = re_compile(r"test-py\{([\w,]+)\}")
python_version_3_regex = re_compile(r"3\d\d?")

root_directory = str(Path(__file__).parents[2])

for root, directories, files in walk(root_directory):
for file in files:
if file == "tox.ini" and root_directory != root:
tox_ini_paths.append(Path(join(root, file)))

config_parser = ConfigParser()

tox_ini_directory_python_versions = []

for tox_ini_path in tox_ini_paths:
config_parser.read(tox_ini_path)

python_versions = []

for env in config_parser["tox"]["envlist"].split():
env_match = env_regex.match(env)

if env_match is not None:
for python_version in env_match.group(1).split(","):

if python_version == "py3":
python_versions.append("pypy-3.8")

elif python_version_3_regex.match(python_version):
python_versions.append(f"3.{python_version[1:]}")

else:
raise Exception(
f"Invalid python version found in {tox_ini_path}: "
f"{python_version}"
)
break

if not python_versions:
raise Exception(f"No python versions found in {tox_ini_path}")

for python_version in python_versions:

tox_ini_directory_python_versions.append(
{
"tox_ini_directory": join(
tox_ini_path.parent.parent.name,
tox_ini_path.parent.name
),
"python_version": python_version
}
)

print(f"tox_ini_directory_python_versions={dumps(tox_ini_directory_python_versions)}")
44 changes: 44 additions & 0 deletions .github/workflows/instrumentations_3.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
name: Tests

on:
push:
branches-ignore:
- 'release/*'
pull_request:

env:
CORE_REPO_SHA: 141a6a2e473ef7f0ec4915dfb71e3c0fa595283e

jobs:

get_tox_ini_directory_python_versions:
name: Get Python versions for each tox_ini_directory
runs-on: ubuntu-latest
outputs:
tox_ini_directory_python_versions: ${{ steps.run_get_tox_ini_directory_python_versions_py.outputs.tox_ini_directory_python_versions }}
steps:
- name: Checkout Contrib Repo @ SHA - ${{ github.sha }}
uses: actions/checkout@v4

- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: "3.12"

- name: Run get_tox_ini_directory_python_versions.py
id: run_get_tox_ini_directory_python_versions_py
run: |
python .github/workflows/get_tox_ini_directory_python_versions.py >> $GITHUB_OUTPUT

run_tests:
name: ${{ matrix.tox_ini_directory_python_versions.tox_ini_directory }} ${{ matrix.tox_ini_directory_python_versions.python_version }} ${{ matrix.os }}
needs: get_tox_ini_directory_python_versions
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-20.04, windows-2019]
tox_ini_directory_python_versions: ${{ fromJson(needs.get_tox_ini_directory_python_versions.outputs.tox_ini_directory_python_versions) }}

steps:
- name: echo stuff
run: echo "${{ matrix.os }} ${{ matrix.tox_ini_directory_python_versions.tox_ini_directory }} ${{ matrix.tox_ini_directory_python_versions.python_version }}"
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,14 @@ venv*/
# Installer logs
pip-log.txt

# Unit test / coverage reports
# Unit test / coverage, benchmark reports
coverage.xml
.coverage
.nox
.tox
.cache
htmlcov
benchmark.json
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file is generated when benchmark tests are run in the local computer of a developer. Added in this file to avoid adding it to git unintentionally.


# Translations
*.mo
Expand Down
2 changes: 1 addition & 1 deletion .isort.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ profile=black
; docs: https://github.com/timothycrosley/isort#multi-line-output-modes
multi_line_output=3
skip=target
skip_glob=**/gen/*,.venv*/*,venv*/*,.tox/*
skip_glob=**/gen/*,.venv*/*,venv*/*,.tox/*,**/.tox/*
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since new tox.ini files are being added, new .tox directories will be generated into every package when tox is executed locally. This tells isort to not check these new .tox directories.

known_first_party=opentelemetry
known_third_party=psutil,pytest,redis,redis_opentracing
2 changes: 2 additions & 0 deletions coverage-requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
coverage==7.5.1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These requirements are only needed for coverate tests, I have removed them from the previous test-requirements.txt files so that we won't install unnecessary dependencies when running regular tests.

pytest-cov==5.0.0
20 changes: 20 additions & 0 deletions lint-requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
astroid==3.1.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here, separated the requirements that are only needed for lint to avoid installing unnecessary packages when running regular tests. Since lint tests are the same for all packages this file is kept in the root directory of this repo to avoid duplicating it in every package.

black==24.4.2
click==8.1.7
Deprecated==1.2.14
dill==0.3.8
flake8==7.0.0
importlib-metadata==7.0.0
isort==5.13.2
mccabe==0.7.0
mypy-extensions==1.0.0
packaging==24.0
pathspec==0.12.1
platformdirs==4.2.1
pycodestyle==2.11.1
pyflakes==3.2.0
pylint==3.1.0
tomlkit==0.12.4
typing_extensions==4.11.0
wrapt==1.16.0
zipp==3.18.1
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,3 @@ tomli==2.0.1
typing_extensions==4.10.0
wrapt==1.16.0
zipp==3.17.0
-e resource/opentelemetry-resource-detector-container
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For uniformity, moved this line to the corresponding tox.ini file.

47 changes: 47 additions & 0 deletions resource/opentelemetry-resource-detector-container/tox.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
[tox]
isolated_build = True
skipsdist = True
skip_missing_interpreters = True
envlist =
test-py{38,39,310,311,py3}
lint
coverage
spellcheck

[testenv]
setenv =
; override CORE_REPO_SHA via env variable when testing other branches/commits than main
; i.e: CORE_REPO_SHA=dde62cebffe519c35875af6d06fae053b3be65ec tox -e <env to test>
CORE_REPO_SHA={env:CORE_REPO_SHA:main}
CORE_REPO=git+https://github.com/open-telemetry/opentelemetry-python.git@{env:CORE_REPO_SHA}

commands_pre =
test,lint,coverage: pip install opentelemetry-api@{env:CORE_REPO}\#egg=opentelemetry-api&subdirectory=opentelemetry-api
test,lint,coverage: pip install opentelemetry-semantic-conventions@{env:CORE_REPO}\#egg=opentelemetry-semantic-conventions&subdirectory=opentelemetry-semantic-conventions
test,lint,coverage: pip install opentelemetry-sdk@{env:CORE_REPO}\#egg=opentelemetry-sdk&subdirectory=opentelemetry-sdk
test,lint,coverage: pip install opentelemetry-test-utils@{env:CORE_REPO}\#egg=opentelemetry-test-utils&subdirectory=tests/opentelemetry-test-utils
test,lint,coverage: pip install -e .

test,coverage: pip install -r {toxinidir}/test-requirements.txt

lint: pip install -r {toxinidir}/../../lint-requirements.txt

coverage: pip install -r {toxinidir}/../../coverage-requirements.txt

spellcheck: pip install -r {toxinidir}/../../spellcheck-requirements.txt

commands =
test: pytest {toxinidir}/tests {posargs}

lint: black --diff --check --config {toxinidir}/../../pyproject.toml {toxinidir}
lint: isort --diff --check-only --settings-path {toxinidir}/../../.isort.cfg {toxinidir}
lint: flake8 --config {toxinidir}/../../.flake8 {toxinidir}
lint: pylint --rcfile={toxinidir}/../../.pylintrc {toxinidir}/src/opentelemetry
lint: pylint --rcfile={toxinidir}/../../.pylintrc {toxinidir}/tests

coverage: coverage erase
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using the coverage commands directly here, so that it is not necessary to check an additional file (scripts/coverage/sh) to understand what coverage tests are doing.

coverage: pytest --cov={toxinidir}/src --cov-append --cov-branch --cov-report='' {toxinidir}/tests
coverage: coverage report --show-missing
coverage: coverage xml

spellcheck: codespell --config {toxinidir}/../../.codespellrc {toxinidir}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

spellcheck can now be run only for a certain package instead for the whole repo, making it faster for most changes.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pytest-benchmark==4.0.0
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ packaging==24.0
pluggy==1.5.0
py-cpuinfo==9.0.0
pytest==7.4.4
pytest-benchmark==4.0.0
tomli==2.0.1
typing_extensions==4.10.0
wrapt==1.16.0
zipp==3.17.0
-e sdk-extension/opentelemetry-sdk-extension-aws
51 changes: 51 additions & 0 deletions sdk-extension/opentelemetry-sdk-extension-aws/tox.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
[tox]
isolated_build = True
skipsdist = True
skip_missing_interpreters = True
envlist =
test-py{38,39,310,311,312,py3}
lint
coverage
spellcheck

[testenv]
setenv =
; override CORE_REPO_SHA via env variable when testing other branches/commits than main
; i.e: CORE_REPO_SHA=dde62cebffe519c35875af6d06fae053b3be65ec tox -e <env to test>
CORE_REPO_SHA={env:CORE_REPO_SHA:main}
CORE_REPO=git+https://github.com/open-telemetry/opentelemetry-python.git@{env:CORE_REPO_SHA}

commands_pre =
test,lint,coverage,benchmark: pip install opentelemetry-api@{env:CORE_REPO}\#egg=opentelemetry-api&subdirectory=opentelemetry-api
test,lint,coverage,benchmark: pip install opentelemetry-semantic-conventions@{env:CORE_REPO}\#egg=opentelemetry-semantic-conventions&subdirectory=opentelemetry-semantic-conventions
test,lint,coverage,benchmark: pip install opentelemetry-sdk@{env:CORE_REPO}\#egg=opentelemetry-sdk&subdirectory=opentelemetry-sdk
test,lint,coverage,benchmark: pip install opentelemetry-test-utils@{env:CORE_REPO}\#egg=opentelemetry-test-utils&subdirectory=tests/opentelemetry-test-utils
test,lint,coverage,benchmark: pip install -e .

test,coverage,benchmark: pip install -r {toxinidir}/test-requirements.txt

lint: pip install -r {toxinidir}/../../lint-requirements.txt

coverage: pip install -r {toxinidir}/../../coverage-requirements.txt

spellcheck: pip install -r {toxinidir}/../../spellcheck-requirements.txt

benchmark: pip install -r {toxinidir}/benchmark-requirements.txt

commands =
test: pytest {toxinidir}/tests {posargs}

lint: black --diff --check --config {toxinidir}/../../pyproject.toml {toxinidir}
lint: isort --diff --check-only --settings-path {toxinidir}/../../.isort.cfg {toxinidir}
lint: flake8 --config {toxinidir}/../../.flake8 {toxinidir}
lint: pylint --rcfile={toxinidir}/../../.pylintrc {toxinidir}/src/opentelemetry
lint: pylint --rcfile={toxinidir}/../../.pylintrc {toxinidir}/tests

coverage: coverage erase
coverage: pytest --cov={toxinidir}/src --cov-append --cov-branch --cov-report='' {toxinidir}/tests
coverage: coverage report --show-missing
coverage: coverage xml

spellcheck: codespell --config {toxinidir}/../../.codespellrc {toxinidir}

benchmark: pytest {toxinidir}/benchmarks --benchmark-json=benchmark.json
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Notice how the benchmark tests are now in their own directory, not inside tests. This makes it possible to run them separately and not run them in CI.

1 change: 1 addition & 0 deletions spellcheck-requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
codespell==2.2.6
7 changes: 0 additions & 7 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,6 @@ deps =
coverage: pytest
coverage: pytest-cov

; FIXME: add coverage testing
; FIXME: add mypy testing
allowlist_externals = sh

Expand Down Expand Up @@ -778,10 +777,6 @@ commands_pre =
util-http: pip install -r {toxinidir}/util/opentelemetry-util-http/test-requirements.txt
util-http: pip install {toxinidir}/util/opentelemetry-util-http

; In order to get a health coverage report,
; we have to install packages in editable mode.
coverage: python {toxinidir}/scripts/eachdist.py install --editable

commands =
test-distro: pytest {toxinidir}/opentelemetry-distro/tests {posargs}
lint-distro: black --diff --check --config {toxinidir}/pyproject.toml {toxinidir}/opentelemetry-distro
Expand Down Expand Up @@ -1137,8 +1132,6 @@ commands =
lint-exporter-prometheus-remote-write: flake8 --config {toxinidir}/.flake8 {toxinidir}/exporter/opentelemetry-exporter-prometheus-remote-write
lint-exporter-prometheus-remote-write: sh -c "cd exporter && pylint --rcfile ../.pylintrc opentelemetry-exporter-prometheus-remote-write"

coverage: {toxinidir}/scripts/coverage.sh
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now, we should not run coverage nor benchmark tests in CI. As they are configured right now, the coverage tests will not fail if a decrease in coverage is detected, the benchmark test will not fail if a decrease in performance is detected. For this reason, they cannot fail CI in a useful manner and it makes no sense to spend resources running them.

For these tests to be usefully executed in CI we need to configure them so that they can fail CI if a decrease in performance (for the benchmark tests) or a decrease in coverage (for the coverage tests) is detected.

Copy link
Member

Choose a reason for hiding this comment

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

👍 makes sense. We should a file a bug regardless of this PR to set up coverage in CI


[testenv:docs]
deps =
-c {toxinidir}/dev-requirements.txt
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
41 changes: 41 additions & 0 deletions workflows_bak/lint_1.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
name: lint 1

on:
push:
branches-ignore:
- 'release/*'
pull_request:
env:
CORE_REPO_SHA: 955c92e91b5cd4bcfb43c39efcef086b040471d2

jobs:
test-1:
env:
RUN_MATRIX_COMBINATION: ${{ matrix.python-version }}-${{ matrix.package }}-${{ matrix.os }}
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false # ensures the entire test matrix is run, even if one permutation fails
matrix:
package:
- "resource/opentelemetry-resource-detector-container"
- "sdk-extension/opentelemetry-sdk-extension-aws"
os: [ubuntu-20.04]
steps:
- name: Checkout Contrib Repo @ SHA - ${{ github.sha }}
uses: actions/checkout@v4
- name: Set up Python ${{ env[matrix.python-version] }}
uses: actions/setup-python@v5
with:
python-version: "3.11"
- name: Install tox
run: pip install tox
- name: Cache tox environment
# Preserves .tox directory between runs for faster installs
uses: actions/cache@v4
with:
path: |
.tox
~/.cache/pip
key: v7-build-tox-cache-${{ env.RUN_MATRIX_COMBINATION }}-${{ hashFiles('tox.ini', 'gen-requirements.txt', 'dev-requirements.txt') }}
- name: run tox
run: tox -c ${{ matrix.package }}/tox.ini -e lint
File renamed without changes.
File renamed without changes.
41 changes: 41 additions & 0 deletions workflows_bak/spellcheck_1.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
name: spellcheck 1

on:
push:
branches-ignore:
- 'release/*'
pull_request:
env:
CORE_REPO_SHA: 955c92e91b5cd4bcfb43c39efcef086b040471d2

jobs:
test-1:
env:
RUN_MATRIX_COMBINATION: ${{ matrix.python-version }}-${{ matrix.package }}-${{ matrix.os }}
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false # ensures the entire test matrix is run, even if one permutation fails
matrix:
package:
- "resource/opentelemetry-resource-detector-container"
- "sdk-extension/opentelemetry-sdk-extension-aws"
os: [ubuntu-20.04]
steps:
- name: Checkout Contrib Repo @ SHA - ${{ github.sha }}
uses: actions/checkout@v4
- name: Set up Python ${{ env[matrix.python-version] }}
uses: actions/setup-python@v5
with:
python-version: "3.11"
- name: Install tox
run: pip install tox
- name: Cache tox environment
# Preserves .tox directory between runs for faster installs
uses: actions/cache@v4
with:
path: |
.tox
~/.cache/pip
key: v7-build-tox-cache-${{ env.RUN_MATRIX_COMBINATION }}-${{ hashFiles('tox.ini', 'gen-requirements.txt', 'dev-requirements.txt') }}
- name: run tox
run: tox -c ${{ matrix.package }}/tox.ini -e spellcheck
File renamed without changes.
Loading