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

Improved test dependency handling, removal of wrapper testing script, and fixes for running pytest directly (issue #644) #781

Merged

Conversation

azaghal
Copy link
Contributor

@azaghal azaghal commented Feb 14, 2018

Most of the changes made in order to satisfy issue #644, with main thing being change of how the dependencies are being handled.

The reasoning in change to test and development requirements handling is to reduce duplication of package listing (so, no need to update in multiple places), to pin the dependent libraries at least to minor version, and to make it a bit more streamlined how test dependencies are installed in development and test environments.

…ling for tests/development (issue django-wiki#644):

- Updated package setup to define cleanly all test requirements within
  it.
- Updated package setup to provide ability to install specific set of
  development/testing/lint testing requirements.
- Removed explicit non-leaf dependencies from test requirements.
- Updated leaf dependencies to target more specific release (current
  stable in most cases). This is primarily fixing versions to specific
  minor release.
- Removed explicit list of dependencies from the tox configuration
  file. Use the dependencies from setup.py instead.
- Updated package setup to support running simple python setup.py test
  for running the tests.
- Updated flake8 configuration to exclude all the standard build
  artefacts, as well as testproject.
- It should be possible to directly run pytest for testing now, as
  well as flake8.
- Updated development documentation centering around how to run tests
  and install dependencies for tests.
- Expanded slightly the development documentation hints for running
  specific tests.
- Removed the runtests.py script.
- Updated manifest file to not include the removed script.
- Updated documentation to reference to direct use of the pytest
  command.
- Updated tox configuration to run pytest directly.
- Added documentation for the SELENIUM_SHOW_BROWSER environment
  variable.
- Updated testing documentation to mention that Xvfb must be available
  on the system as well for runing Selenium tests.
):

- Set the usedevelop value to false, since it is considered dangerous
  to assume that the development environment is the same as the
  distributed one (same set of files etc may not be present).
@codecov
Copy link

codecov bot commented Feb 14, 2018

Codecov Report

Merging #781 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #781   +/-   ##
=======================================
  Coverage   70.02%   70.02%           
=======================================
  Files         100      100           
  Lines        4460     4460           
=======================================
  Hits         3123     3123           
  Misses       1337     1337

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 9c38174...c2cadaa. Read the comment docs.

@azaghal
Copy link
Contributor Author

azaghal commented Feb 14, 2018

Ok, something looks bonkers with coverage, I'll have to look into it :)

@azaghal
Copy link
Contributor Author

azaghal commented Feb 14, 2018

Ok, this is related to removal of usedevelop = true from tox.ini.

@azaghal
Copy link
Contributor Author

azaghal commented Feb 14, 2018

Ok, fixed the issue with coverage not being run. Now, one thing popped to my mind - I could have tox as part of development requirements (this way you don't need to do pip tox install in your dev environment). Should I go ahead and make modification for this too? For test environments one would still need to install tox, though (which kinda makes sense).


usedevelop = true
usedevelop = false
Copy link
Member

Choose a reason for hiding this comment

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

This is a very important addition!! Great work!!

'devel': development_requirements,
'test': test_requirements,
'testlint': test_lint_requirements,
}
Copy link
Member

Choose a reason for hiding this comment

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

I love this pattern!! Really nice overview of requirements, everything maintained in the same place, and the loosest form of dependencies that affect distribution and testing at the same time.

]

test_lint_requirements = [
'flake8>=3.5,<3.6',
Copy link
Member

Choose a reason for hiding this comment

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

Agreed! Actually flake8 has been introducing changes that broke linting out of no where... it's best to be explicit about which version of flake8 that linting is supposed to conform to (together with specs in setup.cfg)

@benjaoming benjaoming merged commit 6fa4c55 into django-wiki:master Feb 15, 2018
@benjaoming benjaoming added this to the 0.4 milestone Feb 15, 2018
@azaghal azaghal deleted the pytest-tox-improvements-issue-644 branch March 6, 2018 20:42
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

Successfully merging this pull request may close these issues.

None yet

2 participants