Skip to content

Fixes in docs#30

Merged
Pringled merged 6 commits intoPringled:mainfrom
barseghyanartur:feature/Test-documentation
Mar 4, 2026
Merged

Fixes in docs#30
Pringled merged 6 commits intoPringled:mainfrom
barseghyanartur:feature/Test-documentation

Conversation

@barseghyanartur
Copy link
Contributor

  • Fixes in docs.
  • Adding pytest-codeblock for testing documentation examples.
  • Adding a new conftest.py for testing documentation examples.
  • Replace irrelevant reference/dependency to pytest-coverage with a valid pytest-cov.
  • Add pytest-cov configuration.
  • Update makefile test command accordingly.

…ples. Adding a new conftest.py for testing documentation examples. Replace irrelevant reference/dependency to pytest-coverage with a valid pytest-cov. Add pytest-cov configuration. Update makefile test command accordingly.
@Pringled
Copy link
Owner

Pringled commented Mar 3, 2026

Hey @barseghyanartur , thanks for adding this! I noticed that CI wasn't running for external contributors, could you please pull main? That should trigger CI. One minor nit: is it possible to move the conftest to a tests file instead? E.g. tests/docs_fixtures.py or something like that? And then you can add it via addopts I believe, e.g. something like

  [tool.pytest.ini_options]
  addopts = [
      "-ra",
      "-q",
      "-p", "tests.docs_fixtures",
      "--ignore=local",
  ]

That way to the top level folder stays a bit cleaner. I can also do that in a followup myself though, let me know.

@Pringled Pringled self-requested a review March 3, 2026 15:30
@barseghyanartur
Copy link
Contributor Author

@Pringled:

Done.

@barseghyanartur
Copy link
Contributor Author

@Pringled:

I actually think it makes sense to separate dev and test (cleaner that way): https://github.com/Pringled/pyversity/blob/main/pyproject.toml#L39

So, dev would still contain things like mypy and ipython, while tests would require all test related deps, like pytest, pytest-cov and pandas.

On CI, you would then install pyversity test option.

Alternatively, pandas could be simply added to dev reqs.

@barseghyanartur
Copy link
Contributor Author

Oops. I see pandas is already in benchmarks. :) So, it could simply be installed on CI as well, before running tests. :)

@codecov
Copy link

codecov bot commented Mar 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.


test:
uv run pytest --cov=PACKAGE --cov-report=term-missing
uv run pytest
Copy link
Owner

Choose a reason for hiding this comment

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

I just noticed this; this is nicer than it was, but I think ci.yaml also needs a small update now. In codecov I see:

Image

I think this is because we now have both coverage run and pytest-cov running. E.g. in CI I see conflicting 100% and 0% coverage tables. However, your approach is better IMO, so I think the nicest thing to do is to update ci.yaml by changing

  # Run tests with coverage
  - name: Run tests under coverage
    run: |
      coverage run --source=pyversity -m pytest
      coverage report

to

  - name: Run tests
    run: uv run pytest 

That should work I think 🤞

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or just make test? :)

Copy link
Owner

Choose a reason for hiding this comment

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

ah yes, forgot about my own make commands 🤦

@Pringled
Copy link
Owner

Pringled commented Mar 4, 2026

LGTM! Thanks for adding this Artur, merging :)

@Pringled Pringled merged commit 50b3743 into Pringled:main Mar 4, 2026
6 checks passed
@barseghyanartur barseghyanartur deleted the feature/Test-documentation branch March 4, 2026 21:43
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