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

This project is great! #259

Closed
1 of 8 tasks
hukkin opened this issue Jun 10, 2021 · 2 comments
Closed
1 of 8 tasks

This project is great! #259

hukkin opened this issue Jun 10, 2021 · 2 comments

Comments

@hukkin
Copy link
Contributor

hukkin commented Jun 10, 2021

Hi @darrenburns, thanks for this project, seems pretty awesome just based on skimming through the docs and repository.

Pytest is great, but Ward seems to fix some of the things that did not feel that great to me. Some things I really like here are

  • tests are not discovered magically, fixtures are not applied magically
  • xpass is failure by default
  • --order random included

There's a few things I could contribute to, but would need to know if a PR is welcome:

  • mypy type checking. I noticed type annotations are already used, but not checked
  • distribute type annotations via PEP561 so type annotations can be benefited from by Ward users
  • pytest has pytest-cov, here it seems coverage run -m ward is the way to go. I'd maybe add a note about that to the docs.
  • Builtin fixtures (I actually noticed there's already an issue). I find myself using tmp_path and capsys by far the most in pytest.
  • Read the config using a TOML v1.0.0 compatible parser (to avoid issues like Valid toml syntax causes black to crash psf/black#2280). I'm very biased here but I'd recommend my own parser Tomli
  • Drop the poetry update from make prep. Instead do the update separately every once in a while. I suggest this because with more contributors just about every PR will be dealing with merge conflicts otherwise.
  • The search word "mock" yielded 0 results in the docs. I assume from unittest import mock is the way to go for most users? Maybe add a note to the docs?
  • Linters don't appear to be running in CI? I'd suggest adding to automate parts of the review process (maybe pre-commit.ci to apply fixes to the PRs automatically?) Ok I found out this is already configured, haha

PS. pytest adds quite a lot of overhead running tests. Would be great to have a design goal to not end up in the same state.

@darrenburns
Copy link
Owner

darrenburns commented Jun 10, 2021

Thanks, I appreciate that! I'll go through your suggestions one by one:

mypy type checking

This would be great and was an issue I've been meaning to create myself. It would be more than just adding a mypy step though, since Ward will currently fail type checking. So far they've been used quite loosely and won't be 100% accurate.

distribute type annotations via PEP561

Agreed!

pytest has pytest-cov, here it seems coverage run -m ward is the way to go. I'd maybe add a note about that to the docs.

Yeah, there are a few areas I'd like to add to the docs include mocking, measuring code coverage, using ward on CI, and comparison between ward and pytest.

I find myself using tmp_path and capsys by far the most in pytest.

Agreed on both of these. There are a couple of places in the Ward suite where we use temporary dirs and tmp_path would've been useful. Let's use the existing issue to track this.

Read the config using a TOML v1.0.0 compatible parser

I wasn't aware of that issue! After looking at Tomli, I'd be happy to accept a PR which switches Ward over to use it.

Drop the poetry update from make prep

Agree :)

The search word "mock" yielded 0 results in the docs.

Answered above

I'll create issues for each of the points above where it makes sense.

@hukkin
Copy link
Contributor Author

hukkin commented Jun 10, 2021

Great, I think there's an issue or PR now for each of the items. Closing this one.

@hukkin hukkin closed this as completed Jun 10, 2021
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

No branches or pull requests

2 participants