Skip to content

Fix global tox test matrix failures and linters - #367

Open
royrhea wants to merge 8 commits into
open-telemetry:mainfrom
royrhea:fix-test-matrix-linters
Open

Fix global tox test matrix failures and linters#367
royrhea wants to merge 8 commits into
open-telemetry:mainfrom
royrhea:fix-test-matrix-linters

Conversation

@royrhea

@royrhea royrhea commented Aug 9, 2026

Copy link
Copy Markdown

Description

This PR fixes issues encountered during local development when running tox environments, and consolidates pytest configuration. It doesn't fix a CI failure, because CI runs in a clean environment, but it improves the local developer experience.

Motivation and context:

  1. Centralize pytest.ini: Removed [tool.pytest.ini_options] from individual instrumentation pyproject.toml files. When packages have their own pytest config, pytest ignores the global pytest.ini. This was causing global settings (like addopts and log_cli) to be ignored. It also allows us to set asyncio_default_fixture_loop_scope = function in one place globally to fix deprecation warnings from pytest-asyncio >= 0.24, instead of repeating it in every package.
  2. Fix shellcheck tox env locally: Updated the tox.ini command for shellcheck to exclude */.tox/* and */.venv/*. Previously, if a developer ran tox -e shellcheck locally after having run other tests, find would locate .sh scripts inside virtual environments and fail. CI passed fine because it always runs in a pristine environment without leftover .tox folders.

Fixes # (No associated issue)

Type of change

  • Bug fix (non-breaking change which fixes an issue - local dev)

How has this been tested?

  • Ran tox -e shellcheck locally with existing .tox and .venv folders, verified it passes instead of failing on dependency shell scripts.
  • Ran pytest locally in an instrumentation package, verified asyncio_default_fixture_loop_scope warnings are resolved and global addopts apply.

Checklist

  • Followed the style guidelines of this project
  • Changelog updated if the change requires an entry (Not required - internal test infra fix)
  • Unit tests added (Not applicable)
  • Documentation updated (Not applicable)

@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Aug 9, 2026

Copy link
Copy Markdown

Pull request dashboard status

Waiting on the author · refreshed 2026-08-24 19:29 UTC

Respond to 1 review item (e.g. link a commit, explain why not, ask a follow-up):

  • Inline threads: 1
Status above doesn't look right?
  • Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
  • Should this be with reviewers? Comment /dashboard route:reviewers to route it to them.
  • Anything wrong — including the routing? Report it with what you expected; it helps us improve the dashboard.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to stabilize repo-wide CI/linting by reducing noise and failures from generated/virtualenv directories and aligning pytest-asyncio behavior with expected fixture scoping.

Changes:

  • Update the shellcheck tox env to skip .tox, .venv, and test_env directories and avoid running shellcheck when no scripts are found.
  • Exclude test_env from the license-header checker and codespell scans.
  • Set pytest-asyncio’s default fixture loop scope to function via pytest.ini.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
tox.ini Adjusts shellcheck invocation and directory exclusions for repo-wide lint stability.
scripts/check_license_header.py Skips test_env to avoid false failures when scanning for SPDX headers.
pytest.ini Sets asyncio_default_fixture_loop_scope to reduce asyncio/fixture-related test failures.
.codespellrc Excludes test_env from spelling checks to avoid scanning generated environments.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tox.ini Outdated
@lmolkova lmolkova added the Skip Changelog PR does not require a changelog entry label Aug 14, 2026
Comment thread pytest.ini
addopts = -rfE -v
log_cli = true
log_cli_level = warning
asyncio_default_fixture_loop_scope = function

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This won't reach the packages that define their own [tool.pytest.ini_options]. pytest stops its config lookup at the package pyproject.toml and never reads root pytest.ini. Affected: anthropic, agno, crewai, smolagents, llama-index, claude-agent-sdk. Those envs will keep emitting the warning.

Simplest fix: drop the [tool.pytest.ini_options] blocks from those six pyproject.tomls so root config applies everywhere. Otherwise the setting has to be duplicated into each.

Comment thread scripts/check_license_header.py Outdated
Comment thread tox.ini Outdated
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Aug 16, 2026

Copy link
Copy Markdown

Hi @royrhea — just a friendly reminder that this pull request is waiting on you. The dashboard status comment has the open items and is kept current.

  • Replying is enough to hand it off — answer, explain why no change is needed, or ask a follow-up. The dashboard routes it onward once nothing on the list is waiting on you.
  • To hand it back for any other reason, including the dashboard getting this wrong, comment /dashboard route:reviewers.

@royrhea

royrhea commented Aug 18, 2026

Copy link
Copy Markdown
Author

@lmolkova Thanks for the feedback! I've updated the PR to address all your suggestions:

  • Test Environment Naming: You were totally right about test_env being an anti-pattern. I've reverted those additions to .codespellrc and the license checker script. We will rely on .gitignore and standard local venv naming instead.
  • Shellcheck POSIX Compliance: Updated the find command for shellcheck to use -print0 and xargs -0 to ensure it handles spaces and is fully POSIX compliant across environments. Also made sure it excludes .venv and venv directories correctly.
  • Pytest Asyncio Scope: I removed the local [tool.pytest.ini_options] blocks from all the individual package pyproject.toml files so that the asyncio_default_fixture_loop_scope = function setting in the root pytest.ini correctly applies repository-wide.

The CI test matrix and linters are all fully green on my end now! Let me know if you need me to make any other changes.

@aabmass

aabmass commented Aug 21, 2026

Copy link
Copy Markdown
Member

@royrhea can you fill out the PR template or file an issue? It's not clear to me what problem this PR solves. The tox tests all pass fine atm.

@royrhea

royrhea commented Aug 24, 2026

Copy link
Copy Markdown
Author

@aabmass I've updated the PR description with the filled-out template. To answer your question, you're right that CI passes fine. This PR fixes issues that only occur during local development (like shellcheck failing on leftover .tox folders) and consolidates our pytest configuration so global options aren't ignored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Skip Changelog PR does not require a changelog entry

Development

Successfully merging this pull request may close these issues.

4 participants