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.

Sign up for GitHub

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

Jump to bottom

#12: Add tests for violation codes and docs #711

Merged
merged 9 commits into from
Oct 12, 2024
Merged

#12: Add tests for violation codes and docs #711

merged 9 commits into from
Oct 12, 2024

Conversation

nifadyev
Copy link
Contributor

@nifadyev nifadyev commented Oct 1, 2024 •
edited
Loading

I have made things!

Checklist

  • I have double checked that there are no unrelated changes in this pull request (old patches, accidental config files, etc)
  • I have created at least one test case for the changes I have made
  • I have updated the documentation for the changes I have made
  • I have added my changes to the CHANGELOG.md

Related issues

Closes #12
Based on PR #88

nifadyev
nifadyev commented Oct 1, 2024
View reviewed changes
tests/test_violations/test_implementation.py Outdated
from dotenv_linter.violations.base import BaseFSTViolation, BaseViolation


def test_visitor_returns_location() -> None:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure, what is the purpose of this. Maybe test method as_line()?

Copy link
Member

Choose a reason for hiding this comment

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

This file can be safely removed, I think

@codecov Codecov
Copy link

codecov bot commented Oct 1, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.28%. Comparing base (460b6fe) to head (e13f5a7).
Report is 96 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #711      +/-   ##
==========================================
- Coverage   97.62%   97.28%   -0.34%     
==========================================
  Files          22       22              
  Lines         463      479      +16     
  Branches       74       95      +21     
==========================================
+ Hits          452      466      +14     
- Misses          8       10       +2     
  Partials        3        3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

sobolevn
sobolevn reviewed Oct 1, 2024
View reviewed changes
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thank you!

CHANGELOG.md Outdated

### Misc

- Add tests for violation codes and docstrings
Copy link
Member

Choose a reason for hiding this comment

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

Tests are our internal stuff, this should not be added to user-facing changelog.

tests/conftest.py Outdated
@@ -11,6 +14,8 @@
BaseViolation,
)

ALL_VIOLATIONS_TYPE = Dict[ModuleType, List[BaseViolation]]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ALL_VIOLATIONS_TYPE = Dict[ModuleType, List[BaseViolation]]
from typing_extensions import TypeAlias
AllViolations: TypeAlias = Dict[ModuleType, List[BaseViolation]]

tests/test_violations/test_implementation.py Outdated
from dotenv_linter.violations.base import BaseFSTViolation, BaseViolation


def test_visitor_returns_location() -> None:
Copy link
Member

Choose a reason for hiding this comment

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

This file can be safely removed, I think

sobolevn
sobolevn reviewed Oct 4, 2024
View reviewed changes
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks! Just one nitpick left

tests/conftest.py Outdated
@@ -11,6 +15,8 @@
BaseViolation,
)

ALL_VIOLATIONS_TYPE: TypeAlias = Dict[ModuleType, List[BaseViolation]]
Copy link
Member

Choose a reason for hiding this comment

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

ALL_VIOLATIONS_TYPE is a constant.
While AllViolationsType is a type :)

Please, rename it.

@nifadyev
Copy link
Contributor Author

nifadyev commented Oct 4, 2024

@sobolevn , could you please help with passing the test test_all_violations_are_final on python 3.9-3.10. Docs on final decorator says that attribute __final__ is available since 3.11. It is the reason the test fails. But cannot find the reason the same test with same python 3.9 passes in wamake-python-styleguide
i've tried import final from typing_extensions, no luck

@sobolevn
Copy link
Member

sobolevn commented Oct 4, 2024

I use typing_extensions for final import. https://github.com/wemake-services/wemake-python-styleguide/blob/master/wemake_python_styleguide/violations/best_practices.py#L176

But, make sure that typing_extensions has the latest version.

@nifadyev nifadyev force-pushed the feature/#12/add-tests-for-violation-codes-and-docs branch from b3c3dc8 to e37f531 Compare October 7, 2024 08:08
@nifadyev
Copy link
Contributor Author

nifadyev commented Oct 7, 2024

Hey @sobolevn , PR fails codecov project check because of unrelated coverage drop (see report). As I can see, file version.py in wemake-python-styleguide differs from the one from dotenv-linter. Maybe the issue with RTD is not relevant no more? I found commit back in 2022 where you dropped this logic for wemake-python-styleguide

sobolevn
sobolevn approved these changes Oct 12, 2024
View reviewed changes
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thank you!

@sobolevn sobolevn merged commit cb170aa into wemake-services:master Oct 12, 2024
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sobolevn sobolevn sobolevn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Test violations codes and docs
2 participants
@nifadyev @sobolevn