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

Refactor: tests to a separated layout, wiki package in src/ #631

Merged
merged 6 commits into from
Apr 21, 2017

Conversation

benjaoming
Copy link
Member

@benjaoming benjaoming commented Apr 21, 2017

Refactor:

  • All test code moved to tests/
  • All of the wiki package now lives in src/
  • Redid some pep8 because some of the codebase was unlinted

From Pytest's Good Integration Practices:

This layout prevents a lot of common pitfalls and has many benefits, which are better explained in this excellent blog post by Ionel Cristian Mărieș.

@benjaoming
Copy link
Member Author

This already caught one error, having had test+development artifacts in the dist tarball/whl on Pypi.

It's definitely going to give a much better guarantee that gaps between local development environments and distributed environments don't get confused in the tests: We test the dist, not the dev environment, as the blog post "Packaging a python library" says.

@benjaoming benjaoming force-pushed the refactor-tests branch 2 times, most recently from fe4583b to 70e2a89 Compare April 21, 2017 16:52
@codecov-io
Copy link

codecov-io commented Apr 21, 2017

Codecov Report

Merging #631 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #631      +/-   ##
==========================================
+ Coverage   67.92%   67.93%   +<.01%     
==========================================
  Files          87       87              
  Lines        4337     4335       -2     
==========================================
- Hits         2946     2945       -1     
+ Misses       1391     1390       -1
Impacted Files Coverage Δ
src/wiki/core/utils.py 100% <ø> (ø)
src/wiki/plugins/help/wiki_plugin.py 0% <ø> (ø)
src/wiki/core/compat.py 80% <ø> (ø)
src/wiki/plugins/attachments/views.py 58.63% <ø> (ø)
src/wiki/plugins/macros/settings.py 100% <ø> (ø)
src/wiki/plugins/attachments/admin.py 100% <ø> (ø)
src/wiki/__init__.py 100% <ø> (ø)
src/wiki/plugins/attachments/__init__.py 100% <ø> (ø)
src/wiki/core/http.py 32.35% <ø> (ø)
src/wiki/plugins/images/views.py 46.08% <ø> (ø)
... and 87 more

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 a235fb2...bc6aa7a. Read the comment docs.

Copy link
Contributor

@benwah benwah left a comment

Choose a reason for hiding this comment

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

One question - Every looks 💯

@@ -64,7 +64,6 @@ def __call__(self, request):
models.URLPath.root().article,
self.request.user):
self.results = self.__filter_can_read(self.request.user)
#self.results = self.results.filter(current_revision__deleted=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

giphy

@@ -0,0 +1,2 @@
from __future__ import absolute_import
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this file needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

As of now, it's needed because a lot of HTML files say {% load wiki_thumbnails %} but I don't know if adding the register from sorl actually makes any difference.. I should try deleting it, you're right!

Copy link
Member Author

Choose a reason for hiding this comment

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

Just verified it, this is actually the reason :) I think it's to provide a decent wrapper for sorl.thumbnail on case it changes API or we add an alternative. Or maybe it's there for some historic reason.

@@ -7,8 +7,8 @@
from wiki.forms import Group
from wiki.models import URLPath

from .base import wiki_override_settings
from .testdata.models import CustomGroup
from ..base import wiki_override_settings
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@benjaoming
Copy link
Member Author

Gonna get this merged now and not let it sit in the way of future PRs!

@benjaoming benjaoming merged commit 7918ff9 into django-wiki:master Apr 21, 2017
@benjaoming benjaoming deleted the refactor-tests branch May 8, 2017 20:44
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.

3 participants