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

Redundant sort test #3359

Closed
korverdev opened this issue Feb 16, 2024 · 3 comments
Closed

Redundant sort test #3359

korverdev opened this issue Feb 16, 2024 · 3 comments

Comments

@korverdev
Copy link
Contributor

I wanted to create an issue to discuss with the maintainers of this repo before I went ahead and deleted a test, but it appears this project is using both a Pre-Commit hook called file-contents-sorter and a test called test_dictionary_sorting() to enforce dictionaries have been sorted.

If I'm not missing anything, since Pre-Commit is enforced as a CI check before merge, the test_dictionary_sorting() test can be deleted. If that's accurate, I can get a PR in deleting the test.

@DimitriPapadopoulos
Copy link
Collaborator

Both were added in #2973. I think the explanation is in a comment:

[...] this is now enforced by the pre-commit hook, and is checked when make check-dictionaries is run.

Can you have a look?

@korverdev
Copy link
Contributor Author

Truth be told, I'm not really following what they're trying to say. Are we sure they were aware that Pre-Commit is enforced as a CI check? From what I can tell, there's really no need to have both.

@DimitriPapadopoulos
Copy link
Collaborator

The Makefile distinguishes between check-dictionaries implemented in actual tests:

codespell/Makefile

Lines 13 to 21 in edef040

check-dictionaries:
@for dictionary in ${DICTIONARIES}; do \
if grep -E -n "^\s*$$|\s$$|^\s" $$dictionary; then \
echo "Dictionary $$dictionary contains leading/trailing whitespace and/or blank lines. Trim with 'make trim-dictionaries'"; \
exit 1; \
fi; \
done
@if command -v pytest > /dev/null; then \
pytest codespell_lib/tests/test_dictionary.py; \

and sort-dictionaries implemented using pre-commit:

codespell/Makefile

Lines 27 to 28 in edef040

sort-dictionaries:
pre-commit run --all-files file-contents-sorter

@korverdev korverdev closed this as not planned Won't fix, can't repro, duplicate, stale May 26, 2024
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

No branches or pull requests

2 participants