Skip to content

test: cover build config-settings and missing backend warning - #11007

Open
Shriprasad-P wants to merge 3 commits into
python-poetry:mainfrom
Shriprasad-P:test/build-command-coverage
Open

Shriprasad-P wants to merge 3 commits into
python-poetry:mainfrom
Shriprasad-P:test/build-command-coverage

Conversation

@Shriprasad-P

Copy link
Copy Markdown

Pull Request Check List

Relates-to: #3155

  • Added tests for changed code.
  • Updated documentation for changed code. (not applicable, test-only change)

Summary

  • Cover the CLI --config-settings local-version=... path end-to-end, including artifact naming and successful exit.
  • Assert the existing --local-version deprecation warning.
  • Assert the No build backend defined warning for projects without a build system/backend.

Why

Issue #3155 still lists poetry build as an outstanding test area. The command already has strong line coverage, so this PR focuses on previously unasserted behavior rather than adding redundant execution-only tests.

Testing

  • poetry run pytest tests/console/commands/test_build.py -q -n0
  • poetry run ruff check tests/console/commands/test_build.py
  • poetry run ruff format --check tests/console/commands/test_build.py
  • poetry run mypy tests/console/commands/test_build.py
  • poetry run pre-commit run --files tests/console/commands/test_build.py

AI assistance

Cursor Agent assisted with repository inspection, identifying the coverage gaps, drafting the tests, and running validation. The contributor reviewed the complete diff and test results before publication.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've found 1 issue, and left some high level feedback:

  • The two local version tests share quite a bit of setup and assertions; consider parametrizing them or extracting a small helper to reduce duplication and make it easier to extend behavior later.
  • The tests for _requires_isolated_build and the no-backend warning are directly calling a private method; if feasible, prefer asserting the behavior via the public build command interface to keep tests resilient to internal refactors.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The two local version tests share quite a bit of setup and assertions; consider parametrizing them or extracting a small helper to reduce duplication and make it easier to extend behavior later.
- The tests for `_requires_isolated_build` and the no-backend warning are directly calling a private method; if feasible, prefer asserting the behavior via the public build command interface to keep tests resilient to internal refactors.

## Individual Comments

### Comment 1
<location path="tests/console/commands/test_build.py" line_range="108-116" />
<code_context>
+        tmp_tester.execute(f"--config-settings local-version={local_version_label}")
+        == 0
+    )
+    build_artifacts = tuple(
+        (tmp_project_path / "dist").glob(
+            get_package_glob(tmp_poetry, local_version=local_version_label)
+        )
+    )
+
+    assert len(build_artifacts) > 0
+    assert all(archive.exists() for archive in build_artifacts)
+    assert "`--local-version` is deprecated." not in tmp_tester.io.fetch_error()


</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen `--config-settings local-version` coverage by asserting artifact names include the local version label

This test currently only verifies that some artifacts exist for the glob from `get_package_glob`, but not that the local version label is actually present in their filenames. If `get_package_glob` became too permissive, the test could still pass while ignoring the local version. Please add an assertion (e.g. `assert all(local_version_label in archive.name for archive in build_artifacts)`) to directly verify artifact naming and better protect against regressions.

Suggested implementation:

```python
    assert len(build_artifacts) > 0
    assert all(archive.exists() for archive in build_artifacts)
    assert all(local_version_label in archive.name for archive in build_artifacts)
    assert "`--local-version` is deprecated." in tmp_tester.io.fetch_error()

```

This change assumes:
1. `build_artifacts` is defined earlier in `test_build_with_config_settings_local_version` as an iterable of artifact paths.
2. `local_version_label` is the string used in the `--config-settings local-version=...` invocation.
If either name differs in your actual file, please adjust the assertion to match the existing variable names.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread tests/console/commands/test_build.py
Shriprasad-P and others added 2 commits August 9, 2026 14:07
Add comprehensive test coverage for build command config-settings:
- Test multiple config-settings passed simultaneously
- Test config-settings with empty values
- Test config-settings with equals signs in values
- Test config-settings combined with deprecated --local-version flag
- Test config-settings passed through to isolated builds
- Test missing build backend warning appears during actual build flow

This brings test count from 17 to 23 tests, strengthening coverage for
build configuration edge cases and ensuring proper warning behavior.

Co-authored-by: Shriprasad R Patil <Shriprasad-P@users.noreply.github.com>

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sourcery assessment

Approved.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants