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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Look in child dirs when discovering django project #586

Closed
wants to merge 2 commits into from
Closed

Look in child dirs when discovering django project #586

wants to merge 2 commits into from

Conversation

voidus
Copy link
Contributor

@voidus voidus commented Mar 11, 2018

Hi! 馃憢
In a lot of projects I work on, the django project is in a src subfolder of the repository. :fol
These changes allow pytest-django to discover the project when pytest is run from the root folder of the repository.

While reading the code, I discovered that py is in maintenance mode, so I replaced it with pathlib, which is in the standard library.

What do you think about these changes?

PS:
I tried to run tox locally, but there were some errors regarding the postgres database. Running one using Docker didn't work either. I also had a little trouble understanding how the django-project-directory-discovery is tested in general, if you feel like this feature deserves a test, please give me some pointers on how to do it.

py is in maintenance mode and there are modern alternatives
@codecov-io
Copy link

Codecov Report

Merging #586 into master will decrease coverage by 0.08%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #586      +/-   ##
==========================================
- Coverage   91.92%   91.84%   -0.09%     
==========================================
  Files          33       33              
  Lines        1660     1667       +7     
  Branches      143      148       +5     
==========================================
+ Hits         1526     1531       +5     
  Misses         95       95              
- Partials       39       41       +2
Flag Coverage 螖
#dj110 83.86% <85.71%> (-0.06%) 猬囷笍
#dj111 86.02% <85.71%> (-0.07%) 猬囷笍
#dj18 84.7% <85.71%> (-0.06%) 猬囷笍
#dj19 83.74% <85.71%> (-0.06%) 猬囷笍
#dj20 84.16% <85.71%> (-0.06%) 猬囷笍
#djmaster 84.16% <85.71%> (-0.06%) 猬囷笍
#mysql_innodb 84.16% <85.71%> (-0.06%) 猬囷笍
#mysql_myisam 84.1% <85.71%> (-0.06%) 猬囷笍
#postgres 87.52% <85.71%> (-0.07%) 猬囷笍
#py27 89.14% <85.71%> (-0.08%) 猬囷笍
#py34 83.74% <85.71%> (-0.06%) 猬囷笍
#py35 83.86% <85.71%> (-0.06%) 猬囷笍
#py36 84.64% <85.71%> (-0.06%) 猬囷笍
#sqlite 85.9% <85.71%> (-0.07%) 猬囷笍
#sqlite_file 83.74% <85.71%> (-0.06%) 猬囷笍
Impacted Files Coverage 螖
tests/conftest.py 100% <100%> (酶) 猬嗭笍
pytest_django/plugin.py 85.42% <82.6%> (-0.34%) 猬囷笍

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 e0f59db...1cdb622. Read the comment docs.

@blueyed
Copy link
Contributor

blueyed commented Mar 23, 2018

I'm -0 on this: I do not like too much magic, which also means overhead - but have this feature altogether disabled for my projects usually anyway.

@voidus
Copy link
Contributor Author

voidus commented Apr 3, 2018

@blueyed Assuming you agree to the code in the first commit, the new functionality is contained in two lines of code, and imo this change makes the auto-discovery much more useful. If you don't like the new feature, I would still suggest the first commit to be included.

Is there anything I can do to move this discussion forward?

@blueyed
Copy link
Contributor

blueyed commented Jul 26, 2018

Moved the first commit to #631.

@blueyed
Copy link
Contributor

blueyed commented Jul 27, 2018

#631 has been merged.

@blueyed
Copy link
Contributor

blueyed commented Jul 30, 2018

btw: #478 - it uses testpaths.

@blueyed
Copy link
Contributor

blueyed commented Sep 16, 2018

Closing for now.

@blueyed blueyed closed this Sep 16, 2018
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

3 participants