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

testing/docs: Include doctests in testing #168

Merged
merged 2 commits into from
Oct 3, 2020
Merged

Conversation

carlpatt
Copy link

@carlpatt carlpatt commented Oct 3, 2020

add a section to "tox.ini" that includes doctests in testing
add doctests to some modules

Changes proposed in this pull request:
I noticed that there were already doctests in some of the modules but they were not being tested. I Think it would be great to include them in testing with the added benefit of the doctests showing examples of how to use the modules.

I got the idea to do this when I found the documentation bug months ago that was patched in this commit

@codecov-commenter
Copy link

codecov-commenter commented Oct 3, 2020

Codecov Report

Merging #168 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #168   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            9         9           
  Lines          560       569    +9     
=========================================
+ Hits           560       569    +9     
Flag Coverage Δ
#GHA_Ubuntu 100.00% <100.00%> (ø)
#GHA_Windows 100.00% <100.00%> (ø)
#GHA_macOS 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/humanize/filesize.py 100.00% <ø> (ø)
src/humanize/number.py 100.00% <ø> (ø)
src/humanize/time.py 100.00% <100.00%> (ø)
tests/test_time.py 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3993a24...8819ece. Read the comment docs.

@hugovk hugovk added changelog: Added For new features testing Unit tests, linting, CI, etc. labels Oct 3, 2020
Copy link
Collaborator

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

src/humanize/number.py Outdated Show resolved Hide resolved
src/humanize/number.py Outdated Show resolved Hide resolved
src/humanize/time.py Outdated Show resolved Hide resolved
src/humanize/filesize.py Show resolved Hide resolved
src/humanize/filesize.py Show resolved Hide resolved
@carlpatt
Copy link
Author

carlpatt commented Oct 3, 2020

@hugovk I took care of all the whitespace issues

'something else'
>>> ordinal(None) is None
True

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, is this newline needed, and those for subsequent ones?

Copy link
Author

@carlpatt carlpatt Oct 3, 2020

Choose a reason for hiding this comment

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

Yes if there isn't a space between the three backticks and the test result doctests report it as a failure for some reason.

for example:

>>> precisedelta(delta, suppress=['seconds', 'milliseconds', 'microseconds'])
    '1.50 minutes'
    ```

Doctests will return:

precisedelta(delta, suppress=['seconds', 'milliseconds', 'microseconds'])
Expected:
'1.50 minutes'
```
Got:
'1.50 minutes'

When there's a blank line between them it passes with no failure.

I kept the...

```pycon 
 *documentation* 
\```

...format because I noticed it in other tests that were accepted before and I'm assuming it's there for highlighting or has something to do with the readthedocs page

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fair enough!

Yes, pycon is for syntax highlighting on RTD.

Thanks, I think this is ready for merge!

@hugovk hugovk merged commit 9d9de69 into jmoiron:master Oct 3, 2020
@carlpatt carlpatt deleted the tests branch October 4, 2020 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: Added For new features testing Unit tests, linting, CI, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants