fix: Fix all test/quality failures in python-library and ensure it stays fixed#355
Merged
fix: Fix all test/quality failures in python-library and ensure it stays fixed#355
Conversation
… tests The base template's Makefile had a call to `pylint ... manage.py *.py` which is inappropriate for some of the cookiecutters, as not all of them create a manage.py. Simply removing the explicit reference is fine, since it's already covered by the `*.py`. This also simplifies the `test_upgrade` and `test_quality` tests for the cookiecutter by running the existing make targets in a virtualenv in the baked output, rather than reproducing the full list of checks in test code. (I haven't tackled the other cookiecutters yet, since some of them actually have test failures if you try to use this approach, or don't yet have a `make test-all` or equivalent.)
nedbat
approved these changes
Jun 13, 2023
Contributor
Author
|
The remaining error was that there weren't any tests for pytest to run, which causes pytest to exit with an error. I've added a placeholder test, copied from the one in cookiecutter-django-app. |
robrap
reviewed
Jun 13, 2023
...ter-python-library/{{cookiecutter.library_name}}/tests/test_{{cookiecutter.library_name}}.py
Outdated
Show resolved
Hide resolved
There's a line in python-template's doc config that expects a _static dir (`html_static_path = ['_static']`) but there's no dir, so docs build breaks for some cookiecutters. This moves the CSS to the base as well so that all cookiecutters can use it.
This currently breaks because 1) there's a `logo_only` reference that breaks in recent sphinx-book-theme, and 2) there's a duplicate "getting help" implicit reference in the README.rst.
Not related to python-library cc tests, but easy fix.
sphinx-book-theme 1.x apparently has an undocumented breaking change in executablebooks/sphinx-book-theme#668 -- `logo_only` is no longer supported, and instead there is a `logo` option that I haven't looked into. I'm not sure what we want, but I'd rather unbreak and move on so that we at least don't have a broken docs build.
The "Getting Help" section creates an implicit duplicate target that conflicts with the `_Getting Help:` reference. Rather than doing something more complicated, just inline the link. I also noticed this "getting started" text was copied into the cookiecutter readmes, which are *instructions* for using the cookiecutters, not used in the template output. This duplicates the root readme in the repo, so I removed that text. (Same for in the repo's own readme.) (This required using a double-underscore link to make it anonymous, otherwise you *still* get an implicit target!) At this point, tests should pass.
Also use this in cookiecutter-django-app, which requires a bit of rework of the template in order to deduplicate the import. (Getting the right number of newlines is a little delicate.)
OK, so that's why the whitespace was managed that way...
timmc-edx
commented
Jun 21, 2023
| ====================== | ||
|
|
||
| Once you have a release, and assuming you have an account there, just go to https://www.djangopackages.com/packages/add/ and add it there. | ||
|
|
Contributor
Author
There was a problem hiding this comment.
Yeah, exactly. (I mentioned this in one of the commits, but I haven't yet updated the PR description to roll up all the change descriptions... need to do that.)
timmc-edx
commented
Jun 21, 2023
| Our real-time conversations are on Slack. You can request a `Slack invitation`_, then join our `community Slack workspace`_. | ||
|
|
||
| For more information about these options, see the `Getting Help`_ page. | ||
| For more information about these options, see the `Getting Help <https://openedx.org/getting-help>`__ page. |
Contributor
Author
There was a problem hiding this comment.
See notes in this commit message: 6c30582 -- happy to have other suggestions, though.
timmc-edx
commented
Jun 21, 2023
timmc-edx
commented
Jun 21, 2023
robrap
reviewed
Jun 21, 2023
Contributor
robrap
left a comment
There was a problem hiding this comment.
Oops. I had pending comments that I forgot to submit.
robrap
approved these changes
Jun 21, 2023
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The output of python-library had a number of lint, quality, and test failures. This commit 1) fixes all of those failures, and 2) ensures that we have test coverage of those quality checks so that we don't regress in the future.
Fixes to quality checks and unit tests:
*.py.html_static_path = ['_static']) but there's no dir in the base, so docs build breaks for python-library (which did not have a copy of the CSS in a _static dir). Moving the CSS to the base fixes this so all cookiecutters can use it.logo_onlyfrom base docs config, as it was breaking the docs build for python-library.logo_onlyis no longer supported, and instead there is alogooption that I haven't looked into. I'm not sure what we want, but I'd rather unbreak and move on so that we at least don't have a broken docs build._Getting Help:reference. Rather than doing something more complicated, just inline the link. (This required using a double-underscore link to make it anonymous, otherwise you still get an implicit target!)Other improvements:
test_upgradeandtest_qualityunit tests to just be a single call tomake upgrade requirements test-all. This allows us to exercise the cookiercutter's Makefile and run all of its quality checks without reproducing the full list of checks in test code. (I haven't tackled the other cookiecutters yet, since some of them actually have test failures if you try to use this approach, or don't yet have amake test-allor equivalent.)--recursivearg from isort calls. (Redundant in isort 5+ and causes a warning.) Not related to python-library cc tests, but easy fix.Merge checklist:
Check off if complete or not applicable: