Skip to content

Commit

Permalink
test: slightly increase coverage, use Fuss classes (andreoliwa#301)
Browse files Browse the repository at this point in the history
* test: check, apply, assert violations, all in the same helper method
* test: some more flake8() and API calls
* test: keep non-breaking space needed by some tests
* test: violations and missing coverage lines
* docs: improve contribution guidelines
* docs: to install hooks / open HTML indexes with Invoke
* refactor: use Python 3.6 type annotations
  • Loading branch information
andreoliwa authored Mar 1, 2021
1 parent 41420fc commit a8846e2
Show file tree
Hide file tree
Showing 26 changed files with 767 additions and 483 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@
- apply changes to setup.cfg ([#288](https://github.com/andreoliwa/nitpick/issues/288)) ([f878630](https://github.com/andreoliwa/nitpick/commit/f87863066642cdab112d3145c488c9a780e7c98d))
- **cli:** add 'ls' command to list configured files ([cfc031b](https://github.com/andreoliwa/nitpick/commit/cfc031bdf30105dec9a8952bfb9657aec939b3b6))
- **cli:** add 'run' command to display violations ([a67bfa8](https://github.com/andreoliwa/nitpick/commit/a67bfa8bdaef2461853a237819cd35622c5935e9))
- **cli:** experimental CLI interface (alpha version) ([#255](https://github.com/andreoliwa/nitpick/issues/255)) ([c9ca5dc](https://github.com/andreoliwa/nitpick/commit/c9ca5dc3cc4586b459e2c58fb2e61d80aa3f1e5d))
- **cli:** filter only the desired files on ls/run commands ([#265](https://github.com/andreoliwa/nitpick/issues/265)) ([f5e4a9c](https://github.com/andreoliwa/nitpick/commit/f5e4a9c47583cd809941ca96ec2ffbdbf0c92c6f))
- drop support for Python 3.5 ([#251](https://github.com/andreoliwa/nitpick/issues/251)) ([9f84a60](https://github.com/andreoliwa/nitpick/commit/9f84a608a4ca02e8a96ec8eaaf55e5cb207b35e3)), closes [#250](https://github.com/andreoliwa/nitpick/issues/250)
- experimental CLI interface (alpha version) ([#255](https://github.com/andreoliwa/nitpick/issues/255)) ([c9ca5dc](https://github.com/andreoliwa/nitpick/commit/c9ca5dc3cc4586b459e2c58fb2e61d80aa3f1e5d))

## [0.23.1](https://github.com/andreoliwa/nitpick/compare/v0.23.0...v0.23.1) (2020-11-02)

Expand Down
39 changes: 21 additions & 18 deletions CONTRIBUTING.rst
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
.. include:: targets.rst

============
Contributing
============
Expand All @@ -18,7 +16,7 @@ Bug reports or feature requests
Documentation improvements
==========================

nitpick could always use more documentation, whether as part of the
Nitpick_ could always use more documentation, whether as part of the
official docs, in docstrings, or even on the web in blog posts,
articles, and such.

Expand All @@ -35,35 +33,41 @@ To set up Nitpick_ for local development:
git clone git@github.com:your_name_here/nitpick.git
cd nitpick

3. Install Poetry_ globally using the recommended way.
3. Install Poetry_ globally using `the recommended way <https://github.com/python-poetry/poetry/#installation>`_.

4. Install packages::
4. Install Invoke_. You can use pipx_ to install it globally: ``pipx install invoke``.

poetry install
5. Install dependencies and pre-commit_ hooks::

# Output:
# Installing dependencies from lock file
# ...
invoke install --hooks

5. Create a branch for local development::
6. Create a branch for local development::

git checkout -b name-of-your-bugfix-or-feature

Now you can make your changes locally.

6. When you're done making changes, run pre-commit checks and tests with::
7. When you're done making changes, run tests and checks locally with::

# Quick tests and checks
make
# Or use this to simulate a full CI build with tox
invoke ci-build

7. Commit your changes and push your branch to GitHub::
8. Commit your changes and push your branch to GitHub::

git add .
git commit -m "feat: detailed description of your changes"

# For a feature:
git commit -m "feat: short description of your feature"
# For a bug fix:
git commit -m "fix: short description of what you fixed"

git push origin name-of-your-bugfix-or-feature

8. Submit a pull request through the GitHub website.
9. Submit a pull request through the GitHub website.

9. If your pull request is accepted, all your commits will be squashed into one, and the `Conventional Commits Format <https://www.conventionalcommits.org/>`_ will be used on the commit message.
10. If your pull request is accepted, all your commits will be squashed into one, and the `Conventional Commits Format <https://www.conventionalcommits.org/>`_ will be used on the commit message.

Pull Request Guidelines
-----------------------
Expand All @@ -72,11 +76,10 @@ If you need some code review or feedback while you're developing the code just m

For merging, you should:

1. Include passing tests (run ``make test``) [1]_.
1. Include passing tests (run ``invoke test``) [1]_.
2. Update documentation when there's new API, functionality etc.
3. Add yourself to ``AUTHORS.rst``.

.. [1] If you don't have all the necessary python versions available locally you can rely on Travis - it will
`run the tests <https://github.com/andreoliwa/nitpick/actions/workflows/python.yaml>`_ for each change you add in the pull request.
.. [1] If you don't have all the necessary python versions available locally you can rely on GitHub Workflows: `tests will run <https://github.com/andreoliwa/nitpick/actions/workflows/python.yaml>`_ for each change you add in the pull request.
It will be slower though ...
4 changes: 2 additions & 2 deletions docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@

# -- Options for LaTeX output ------------------------------------------------

latex_elements = {
latex_elements: Dict[str, str] = {
# The paper size ('letterpaper' or 'a4paper').
#
# 'papersize': 'letterpaper',
Expand All @@ -166,7 +166,7 @@
# Latex figure (float) alignment
#
# 'figure_align': 'htbp',
} # type: Dict[str, str]
}

# Grouping the document tree into LaTeX files. List of tuples
# (source start file, target name, title,
Expand Down
2 changes: 2 additions & 0 deletions docs/contributing.rst
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
.. include:: targets.rst

.. include:: ../CONTRIBUTING.rst
19 changes: 10 additions & 9 deletions docs/targets.rst
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
.. _nitpick-style.toml:
.. _default style file: https://raw.githubusercontent.com/andreoliwa/nitpick/v0.24.1/nitpick-style.toml

.. _Bash: https://www.gnu.org/software/bash/
.. _black: https://github.com/psf/black
.. _commitlint: https://commitlint.js.org/
.. _default style file: https://raw.githubusercontent.com/andreoliwa/nitpick/v0.24.1/nitpick-style.toml
.. _Django: https://www.djangoproject.com
.. _flake8: https://gitlab.com/pycqa/flake8/
.. _Flask CLI: https://flask.palletsprojects.com/en/1.1.x/cli/
.. _Invoke: https://github.com/pyinvoke/invoke
.. _IPython: https://ipython.org
.. _isort: https://github.com/PyCQA/isort/
.. _mypy: https://github.com/python/mypy/
.. _nitpick-style.toml:
.. _Nitpick: https://github.com/andreoliwa/nitpick/
.. _package.json: https://docs.npmjs.com/files/package.json
.. _Pipenv: https://github.com/pypa/pipenv/
.. _pipx: https://github.com/pipxproject/pipx
.. _Poetry: https://github.com/python-poetry/poetry/
.. _pre-commit: https://pre-commit.com/
.. _Pylint: https://www.pylint.org
.. _TOML: https://github.com/toml-lang/toml
.. _IPython: https://ipython.org
.. _package.json: https://docs.npmjs.com/files/package.json
.. _Bash: https://www.gnu.org/software/bash/
.. _commitlint: https://commitlint.js.org/
.. _pytest: https://pytest.org/
.. _pyproject-toml-poetry: https://github.com/python-poetry/poetry/blob/master/docs/docs/pyproject.md
.. _pytest: https://pytest.org/
.. _TOML: https://github.com/toml-lang/toml
6 changes: 3 additions & 3 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ setenv =
PY_IGNORE_IMPORTMISMATCH = 1
commands =
python -m pip --version
# https://pytest-cov.readthedocs.io/en/latest/config.html#caveats
# https://docs.pytest.org/en/stable/skipping.html
python -m pytest --cov-config=setup.cfg --cov --cov-append --cov-report=term-missing --doctest-modules -s -rxXs {posargs:-vv}

Expand All @@ -71,10 +72,10 @@ description = Lint all files with pre-commit
basepython = python3.8
platform = linux|darwin
# pylint needs both these extras:
# lint: install pylint itself
# test: for pylint to inspect tests
extras =
# Install pylint itself
lint
# For pylint to inspect tests
test
deps =
pip>=19.2
Expand All @@ -100,7 +101,6 @@ description = Build the HTML docs using Sphinx (sphinx-build, API docs, link che
basepython = python3.8
platform = linux|darwin
extras = doc
# Options to debug Sphinx: -nWT --keep-going -vvv
commands =
sphinx-apidoc --force --module-first --separate --implicit-namespaces --output-dir docs/source src/nitpick/
python3 docs/generate_rst.py
Expand Down
10 changes: 2 additions & 8 deletions src/nitpick/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import click
from loguru import logger

from nitpick.constants import PROJECT_NAME
from nitpick.exceptions import QuitComplainingError
from nitpick.generic import filter_names, relative_to_current_dir
from nitpick.plugins.info import FileInfo
Expand Down Expand Up @@ -99,9 +98,8 @@ def enforce_present_absent(self, *partial_names: str) -> Iterator[Fuss]:
def enforce_style(self, *partial_names: str, apply=True) -> Iterator[Fuss]:
"""Read the merged style and enforce the rules in it.
1. Get all root keys from the merged style
2. All except "nitpick" are file names.
3. For each file name, find the plugin(s) that can handle the file.
1. Get all root keys from the merged style (every key is a filename, except "nitpick").
2. For each file name, find the plugin(s) that can handle the file.
:param partial_names: Names of the files to enforce configs for.
:param apply: Flag to apply changes, if the plugin supports it (default: True).
Expand All @@ -114,10 +112,6 @@ def enforce_style(self, *partial_names: str, apply=True) -> Iterator[Fuss]:
logger.info(f"{config_key}: Finding plugins to enforce style")

# 2.
if config_key == PROJECT_NAME:
continue

# 3.
info = FileInfo.create(self.project, config_key)
# pylint: disable=no-member
for plugin_class in self.project.plugin_manager.hook.can_handle(info=info): # type: Type[NitpickPlugin]
Expand Down
16 changes: 7 additions & 9 deletions src/nitpick/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,16 @@ def pre_commit_without_dash(path_from_root: str) -> bool:
return False

@staticmethod
def jsonfile_section(style_errors: Dict[str, Any], is_merged_style: bool) -> bool:
def jsonfile_section(style_errors: Dict[str, Any]) -> bool:
"""The [nitpick.JSONFile] is not needed anymore; JSON files are now detected by the extension."""
has_nitpick_jsonfile_section = style_errors.get(PROJECT_NAME, {}).pop("JSONFile", None)
if has_nitpick_jsonfile_section:
if not style_errors[PROJECT_NAME]:
style_errors.pop(PROJECT_NAME)
if not is_merged_style:
warnings.warn(
"The [nitpick.JSONFile] section is not needed anymore; just declare your JSON files directly",
DeprecationWarning,
)
return True
style_errors.pop(PROJECT_NAME)
warnings.warn(
"The [nitpick.JSONFile] section is not needed anymore; just declare your JSON files directly",
DeprecationWarning,
)
return True
return False


Expand Down
16 changes: 8 additions & 8 deletions src/nitpick/formats.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ def __init__(

self.format_class = format_class

self.missing = None # type: Optional[BaseFormat]
self.missing_dict = {} # type: Union[JsonDict, YamlData]
self.missing: Optional[BaseFormat] = None
self.missing_dict: Union[JsonDict, YamlData] = {}

self.diff = None # type: Optional[BaseFormat]
self.diff_dict = {} # type: Union[JsonDict, YamlData]
self.diff: Optional[BaseFormat] = None
self.diff_dict: Union[JsonDict, YamlData] = {}

@property
def has_changes(self) -> bool:
Expand All @@ -45,7 +45,7 @@ def has_changes(self) -> bool:
@staticmethod
def _normalize_value(value: Union[JsonDict, YamlData, "BaseFormat"]) -> JsonDict:
if isinstance(value, BaseFormat):
dict_value = value.as_data # type: JsonDict
dict_value: JsonDict = value.as_data
else:
dict_value = value
return flatten(dict_value)
Expand Down Expand Up @@ -110,7 +110,7 @@ def __init__(
raise RuntimeError("Inform at least one argument: path, string or data")

self._ignore_keys = ignore_keys or []
self._reformatted = None # type: Optional[str]
self._reformatted: Optional[str] = None
self._loaded = False

@abc.abstractmethod
Expand Down Expand Up @@ -147,10 +147,10 @@ def _create_comparison(self, expected: Union[JsonDict, YamlData, "BaseFormat"]):
if not self._ignore_keys:
return Comparison(self.as_data or {}, expected or {}, self.__class__)

actual_original = self.as_data or {} # type: Union[JsonDict, YamlData]
actual_original: Union[JsonDict, YamlData] = self.as_data or {}
actual_copy = actual_original.copy() if isinstance(actual_original, dict) else actual_original

expected_original = expected or {} # type: Union[JsonDict, YamlData, "BaseFormat"]
expected_original: Union[JsonDict, YamlData, "BaseFormat"] = expected or {}
if isinstance(expected_original, dict):
expected_copy = expected_original.copy()
elif isinstance(expected_original, BaseFormat):
Expand Down
9 changes: 1 addition & 8 deletions src/nitpick/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,11 @@ def flatten(dict_, parent_key="", separator=".", current_lists=None) -> JsonDict
Use :py:meth:`unflatten()` to revert.
Adapted from `this StackOverflow question <https://stackoverflow.com/a/6027615>`_.
>>> expected = {'root.sub1': 1, 'root.sub2.deep': 3, 'sibling': False}
>>> flatten({"root": {"sub1": 1, "sub2": {"deep": 3}}, "sibling": False}) == expected
True
>>> expected = {'parent."with.dot".again': True, 'parent."my.my"': 1, "parent.123": "numeric-key"}
>>> flatten({"parent": {"with.dot": {"again": True}, "my.my": 1, 123: "numeric-key"}}) == expected
True
"""
if current_lists is None:
current_lists = {}

items = [] # type: List[Tuple[str, Any]]
items: List[Tuple[str, Any]] = []
for key, value in dict_.items():
quoted_key = f"{DOUBLE_QUOTE}{key}{DOUBLE_QUOTE}" if separator in str(key) else key
new_key = str(parent_key) + separator + str(quoted_key) if parent_key else quoted_key
Expand Down
25 changes: 8 additions & 17 deletions src/nitpick/plugins/pre_commit.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ class Violations(ViolationEnum):
REPO_DOES_NOT_EXIST = (333, ": repo {repo!r} does not exist under {key!r}")
MISSING_KEY_IN_REPO = (334, ": missing {key!r} in repo {repo!r}")
STYLE_FILE_MISSING_NAME = (335, ": style file is missing {key!r} in repo {repo!r}")
MISSING_KEY_IN_HOOK = (336, ": style file is missing {key!r} in hook:\n{yaml}")
MISSING_HOOK_WITH_ID = (337, ": missing hook with id {id!r}:\n{yaml}")
MISSING_KEY_IN_HOOK = (336, ": style file is missing {key!r} in hook:")
MISSING_HOOK_WITH_ID = (337, ": missing hook with id {id!r}:")


class PreCommitPlugin(NitpickPlugin):
Expand All @@ -94,15 +94,10 @@ def initial_contents(self) -> str:
original_repos = original.pop(KEY_REPOS, [])
suggested: Dict[str, Any] = {KEY_REPOS: []} if original_repos else {}
for repo in original_repos:
new_repo = dict(repo)
hooks_or_yaml = repo.get(KEY_HOOKS, repo.get(KEY_YAML, {}))
if KEY_YAML in repo:
repo_list = YAMLFormat(string=hooks_or_yaml).as_list
suggested[KEY_REPOS].extend(repo_list)
else:
# TODO: show a deprecation warning for this case
new_repo[KEY_HOOKS] = YAMLFormat(string=hooks_or_yaml).as_data
suggested[KEY_REPOS].append(new_repo)
if KEY_YAML not in repo:
continue
repo_list = YAMLFormat(string=repo[KEY_YAML]).as_list
suggested[KEY_REPOS].extend(repo_list)
suggested.update(original)
return YAMLFormat(data=suggested).reformatted

Expand Down Expand Up @@ -176,7 +171,6 @@ def enforce_repo_old_format(self, index: int, repo_data: OrderedDict) -> Iterato
yield self.reporter.make_fuss(Violations.MISSING_KEY_IN_REPO, key=KEY_HOOKS, repo=repo_name)
return

actual_hooks = actual_repo_dict.get(KEY_HOOKS) or []
yaml_expected_hooks = repo_data.get(KEY_HOOKS)
if not yaml_expected_hooks:
yield self.reporter.make_fuss(Violations.STYLE_FILE_MISSING_NAME, key=KEY_HOOKS, repo=repo_name)
Expand All @@ -187,13 +181,10 @@ def enforce_repo_old_format(self, index: int, repo_data: OrderedDict) -> Iterato
hook_id = expected_dict.get(KEY_ID)
expected_yaml = self.format_hook(expected_dict).rstrip()
if not hook_id:
yield self.reporter.make_fuss(Violations.MISSING_KEY_IN_HOOK, key=KEY_ID, yaml=expected_yaml)
yield self.reporter.make_fuss(Violations.MISSING_KEY_IN_HOOK, expected_yaml, key=KEY_ID)
continue

actual_dict = find_object_by_key(actual_hooks, KEY_ID, hook_id)
if not actual_dict:
yield self.reporter.make_fuss(Violations.MISSING_HOOK_WITH_ID, id=hook_id, yaml=expected_yaml)
continue
yield self.reporter.make_fuss(Violations.MISSING_HOOK_WITH_ID, expected_yaml, id=hook_id)

@staticmethod
def format_hook(expected_dict) -> str:
Expand Down
8 changes: 3 additions & 5 deletions src/nitpick/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,11 @@ def flatten_marshmallow_errors(errors: Dict) -> str:
"""Flatten Marshmallow errors to a string."""
formatted = []
for field, data in SortedDict(flatten(errors)).items():
if isinstance(data, list):
if isinstance(data, (list, tuple)):
messages_per_field = [f"{field}: {', '.join(data)}"]
elif isinstance(data, dict):
messages_per_field = [f"{field}[{index}]: {', '.join(messages)}" for index, messages in data.items()]
else:
# This should never happen; if it does, let's just convert to a string
messages_per_field = [str(errors)]
# This should not happen; if it does, let's just convert to a string
messages_per_field = [f"{field}: {data}"]
formatted.append("\n".join(messages_per_field))
return "\n".join(formatted)

Expand Down
Loading

0 comments on commit a8846e2

Please sign in to comment.