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

Add specific testing instructions to contributing.rst #3074

Closed
wants to merge 8 commits into from

Conversation

jeremycarroll
Copy link

@jeremycarroll jeremycarroll commented Oct 22, 2018

The issue

I wanted to start contributing to the project, the instruction "Run the tests to confirm they all pass on your system" is non-trivial, and took longer than really needed to put into effect.

The fix

I documented what I did to perform this task.

Open Concern

The first way I suggest running the tests is make test; however, this does not work for me.
I don't know what it would take to have success, but believe it is a legitimate way to achieve the goal.

The checklist

  • [None ] Associated issue
  • [Not appropriate] A news fragment in the news/ directory to describe this fix with the extension .bugfix, .feature, .behavior, .doc. .vendor. or .trivial (this will appear in the release changelog). Use semantic line breaks and name the file after the issue number or the PR #.

3. Using pipenv::

pipenv install --dev
pipenv run py.test
Copy link
Member

Choose a reason for hiding this comment

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

I believe pytest is recommended over py.test by Pytest folks now. Also you might want to consider Windows for these instructions :) Most of these won’t work on Windows.

Copy link
Author

@jeremycarroll jeremycarroll Oct 24, 2018

Choose a reason for hiding this comment

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

I changed to pytest as suggested.

OTOH, I am not inclined (or able) to also document what is needed for Windows (other than mentioning run-tests.bat). I agree that the example config instructions won't work for most people. They are explicitly an example of what might be required, and I think they will be helpful even for people who have to modify them. I also think more examples from more platforms would be good.

@techalchemy
Copy link
Member

this PR looks good, lets use it as a starting point, although I'm 99% sure that run-tests.bat doesn't work on windows. I'm sure we will hear about it to confirm :)

@techalchemy techalchemy added Type: Documentation 📖 This issue relates to documentation of pipenv. PR: awaiting-merge The PR related to this issue has been reviewed and is awaiting merge. labels Oct 30, 2018
techalchemy added a commit that referenced this pull request Oct 30, 2018
Signed-off-by: Dan Ryan <dan@danryan.co>
@uranusjr
Copy link
Member

Yeah, once we put it in the docs people will come asking if it does not work :p

techalchemy added a commit that referenced this pull request Oct 30, 2018
Signed-off-by: Dan Ryan <dan@danryan.co>
@jeremycarroll jeremycarroll deleted the dev-doc-testing branch November 3, 2018 03:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: awaiting-merge The PR related to this issue has been reviewed and is awaiting merge. Type: Documentation 📖 This issue relates to documentation of pipenv.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants