Skip to content

Add unit tests #69

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 26, 2021
Merged

Add unit tests #69

merged 2 commits into from
Oct 26, 2021

Conversation

jkittner
Copy link
Contributor

Hi,
I started implementing some unit tests for this module. I hope this is okay...
If I find any bugs during this process, I will send a separate patch and rebase this branch onto the main branch.

As you can see, this is just a draft and still work in progress. I am aiming for full coverage which may take a while.

I am unsure how your licensing conventions work, please let me know if there is something wrong.

@tannewt
Copy link
Member

tannewt commented Sep 28, 2021

Looks great! Thanks for checking in. I'd love to merge these in. Let's make sure they run in GitHub Actions too.

@jkittner
Copy link
Contributor Author

thanks, I added a few more tests and also modified gha to run the tests, which seems to work, however I am not sure if this is the way you like them to run and if we want to to tests against different version of (C)Python ?!

@jkittner jkittner mentioned this pull request Sep 29, 2021
Closed
20 tasks
@tannewt
Copy link
Member

tannewt commented Sep 30, 2021

Looks all good to me! I think you have a better understanding of best test practices than I do. Thanks!

@jkittner jkittner mentioned this pull request Oct 20, 2021
Merged
@jkittner
Copy link
Contributor Author

This should be ready for review now.
I got up to 90 % coverage, the missing 10 % are mainly caused by the inherited I2C class, where I wasn't able to find a proper way of mocking this without actually having a device connected via I2C -- I am open for any hints here.
Some other missing coverage is caused by small one line branches, where I am not even sure, if the case can be ever true?

The CI is failing, because of the bug fixed in #70 which is coverd by one test, so I'd suggest once the bug fix is merged, I will rebase this branch onto master and then it should be ready to merge with a passing CI.

Would you mind adding the hacktoberfest-accepted label to the PRs I made? I'd very much appreciate that 😄

@jkittner jkittner marked this pull request as ready for review October 20, 2021 20:02
tannewt
tannewt reviewed Oct 21, 2021
View reviewed changes
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Looks good to me. Just needs that other fix. Thank you!

tannewt
tannewt approved these changes Oct 26, 2021
View reviewed changes
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thank you!

@tannewt tannewt merged commit b5b18fa into adafruit:main Oct 26, 2021
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Oct 29, 2021
@adafruit-adabot
Automated update by Adabot (adafruit/adabot@61b0168)
895eb32 
Updating https://github.com/adafruit/Adafruit_CircuitPython_ADS1x15 to 2.2.9 from 2.2.8:
  > Merge pull request adafruit/Adafruit_CircuitPython_ADS1x15#73 from tekktrik/feature/add-typing
  > add docs link to readme
  > Globally disabled consider-using-f-string pylint check
  > Moved default branch to main
  > Moved CI to Python 3.7
  > Added help text and problem matcher
  > Added pull request template
  > "Increase duplicate code check threshold "

Updating https://github.com/adafruit/Adafruit_CircuitPython_ESP_ATcontrol to 0.5.8 from 0.5.7:
  > Merge pull request adafruit/Adafruit_CircuitPython_ESP_ATcontrol#44 from PontusO/revert-41-main
  > Merge pull request adafruit/Adafruit_CircuitPython_ESP_ATcontrol#45 from jsymons/typing
  > Merge pull request adafruit/Adafruit_CircuitPython_ESP_ATcontrol#46 from adafruit/not-using-with
  > add docs link to readme
  > Globally disabled consider-using-f-string pylint check

Updating https://github.com/adafruit/Adafruit_CircuitPython_GPS to 3.9.4 from 3.9.3:
  > Merge pull request adafruit/Adafruit_CircuitPython_GPS#69 from theendlessriver13/add-tests
  > Merge pull request adafruit/Adafruit_CircuitPython_GPS#70 from theendlessriver13/fix-byte-indexing

Updating https://github.com/adafruit/Adafruit_CircuitPython_PCF8523 to 1.5.6 from 1.5.5:
  > Merge pull request adafruit/Adafruit_CircuitPython_PCF8523#24 from tekktrik/feature/add-typing
  > add docs link to readme
  > Globally disabled consider-using-f-string pylint check
  > Moved default branch to main
  > Moved CI to Python 3.7
  > Added help text and problem matcher
  > Added pull request template

Updating https://github.com/adafruit/Adafruit_CircuitPython_AzureIoT to 2.4.1 from 2.4.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_AzureIoT#34 from ronnie-llamado/feature/add-type-annotations
  > add docs link to readme
  > Globally disabled consider-using-f-string pylint check
  > Moved default branch to main
  > Moved CI to Python 3.7
  > Added help text and problem matcher
  > Added pull request template
@jkittner jkittner deleted the add-tests branch November 10, 2021 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tannewt tannewt tannewt approved these changes

Assignees
No one assigned
Labels
hacktoberfest-accepted
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jkittner @tannewt