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

Tests README: Add instructions to install testing-related packages #1180

Merged
merged 7 commits into from
Aug 1, 2022

Conversation

mathieuboudreau
Copy link
Contributor

@mathieuboudreau mathieuboudreau commented Jul 11, 2022

Checklist

GitHub

  • I've given this PR a concise, self-descriptive, and meaningful title
  • I've linked relevant issues in the PR body
  • I've applied the relevant labels to this PR
  • I've assigned a reviewer

PR contents

Description

In the testing/ README, it was previously assumed that you already had pytest and pytest_console_scripts already installed, but these are not installed along with the IVADOMED requirements. I've added instructions to install them, otherwise users might get an error (I did).

Linked issues

@coveralls
Copy link

coveralls commented Jul 22, 2022

Pull Request Test Coverage Report for Build 2770497171

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+9.8%) to 78.975%

Totals Coverage Status
Change from base Build 2770265565: 9.8%
Covered Lines: 4838
Relevant Lines: 6126

💛 - Coveralls

@dyt811
Copy link
Member

dyt811 commented Jul 31, 2022

Good call. I agree. This was actually because we USED to have a requirement file for those for dev specifically but since a recent PR streamlining the installation process, it has been borked.

Copy link
Member

@dyt811 dyt811 left a comment

Choose a reason for hiding this comment

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

Thanks!

testing/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@kousu kousu 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 running through the install process and catching these problems. These are the sorts of things that fall through the cracks when people write software together. See my comments abovebelow for what I think should be done.

I'm sorry it got lost in my inbox @mathieuboudreau.

testing/README.md Outdated Show resolved Hide resolved
testing/README.md Outdated Show resolved Hide resolved
@kanishk16 kanishk16 added the documentation category: readthedocs or user doc label Jul 31, 2022
Copy link
Contributor

@kanishk16 kanishk16 left a comment

Choose a reason for hiding this comment

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

Thanks @mathieuboudreau as we would have definitely missed this! LGTM apart from a minor suggestion which is continuation to Nick's comment.

testing/README.md Outdated Show resolved Hide resolved
@mathieuboudreau
Copy link
Contributor Author

Thank you all for your reviews and suggestions! Merging!

@mathieuboudreau mathieuboudreau merged commit f9653c0 into ivadomed:master Aug 1, 2022
@mathieuboudreau mathieuboudreau deleted the mb/testing-doc branch August 1, 2022 01:52
@mariehbourget mariehbourget added this to the new release milestone Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation category: readthedocs or user doc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants