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

Make tests run against local code, fix syntax warning breaking coverage, and add code coverage generation to github actions #140

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

scott-carroll-verses-ai
Copy link

@scott-carroll-verses-ai scott-carroll-verses-ai commented Jun 12, 2024

  • Update tests so they test against local code rather than the installed module
  • Update syntax so the warning around ellipsis use doesn't break coverage generation
  • Update github actions to produce a coverage report and attach it to the run

See:

Related to issue #142

@scott-carroll-verses-ai scott-carroll-verses-ai changed the title Fix tests add coverage Make tests run against local code, fix syntax warning breaking coverage, and add code coverage generation to github actions Jun 12, 2024
@conorheins conorheins self-requested a review July 1, 2024 16:24
@tverbele
Copy link
Collaborator

@scott-carroll-verses-ai why should we hack the sys.path for local test execution as opposed to installing the library locally using pip install -e . and then running pytest?

@scott-carroll-verses-ai
Copy link
Author

@scott-carroll-verses-ai why should we hack the sys.path for local test execution as opposed to installing the library locally using pip install -e . and then running pytest?

Good question @tverbele . The idea is that you test the code before you install it. Can help with issues during dev re: e.g. needing to wipe out cache and change versions of the lib as you work on the "next" version. i.e. to avoid installing a hacky implementation before testing. YMMV 🤷‍♀️

Happy to chat more but my feelings won't be hurt if you decide to not test that way.

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