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

Additional Pycodestyle Checker Tests - E304, 303, 301, 275, 266 #1037

Merged
merged 22 commits into from
May 30, 2024
Merged

Additional Pycodestyle Checker Tests - E304, 303, 301, 275, 266 #1037

merged 22 commits into from
May 30, 2024

Conversation

lana-w
Copy link
Contributor

@lana-w lana-w commented May 21, 2024 •
edited by david-yz-liu
Loading

Proposed Changes

This pull requests adds the following tests to the test_pycodestyle_checker suite to increase the coverage of PycodestyleChecker:

  • E266: Too many leading '#' for block comment
  • E275: Missing whitespace after keyword
  • E301: Expected 1 blank line, found 0
  • E303: Too many blank lines (3)
  • E304: Blank line found after function decorator

Type of Change

Type Applies?
🚨 Breaking change (fix or feature that would cause existing functionality to change)
✨ New feature (non-breaking change that adds functionality)
🐛 Bug fix (non-breaking change that fixes an issue)
♻️ Refactoring (internal change to codebase, without changing functionality)
🚦 Test update (change that only adds or modifies tests) X
📚 Documentation update (change that only updates documentation)
📦 Dependency update (change that updates a dependency)
🔧 Internal (change that only affects developers or continuous integration)

Checklist

(Complete each of the following items for your pull request. Indicate that you have completed an item by changing the [ ] into a [x] in the raw text, or by clicking on the checkbox in the rendered description on GitHub.)

Before opening your pull request:

  • I have performed a self-review of my changes.
    • Check that all changed files included in this pull request are intentional changes.
    • Check that all changes are relevant to the purpose of this pull request, as described above.
  • I have added tests for my changes, if applicable.
    • This is required for all bug fixes and new features.
  • I have updated the project documentation, if applicable.
    • This is required for new features.
  • I have updated the project Changelog (this is required for all changes).
  • If this is my first contribution, I have added myself to the list of contributors.

After opening your pull request:

  • I have verified that the pre-commit.ci checks have passed.
  • I have verified that the CI tests have passed.
  • I have reviewed the test coverage changes reported by Coveralls.
  • I have requested a review from a project maintainer.

Questions and Comments

@coveralls
Copy link
Collaborator

coveralls commented May 21, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9296652307

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+1.3%) to 92.92%
Totals Coverage Status
Change from base Build 9230688860: 1.3%
Covered Lines: 2848
Relevant Lines: 3065
💛 - Coveralls

@lana-w lana-w requested a review from david-yz-liu May 21, 2024 01:26
david-yz-liu
david-yz-liu reviewed May 21, 2024
View reviewed changes
Copy link
Contributor

@david-yz-liu david-yz-liu left a comment

Choose a reason for hiding this comment

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

@lana-w good work, I just left a few minor comments. Also, since I merged in #1036 you'll need to resolve some merge conflicts. Before doing a pull from upstream master, I recommend changing the git diff algorithm in the git configuration, described in the "Using Git Effectively" page. That will hopefully help reduce the number of merge conflicts.

tests/test_custom_checkers/test_pycodestyle_checker.py Outdated
@@ -100,7 +100,107 @@ def test_error_e262(self) -> None:
self.checker.process_module(mod)

def test_no_error_e262(self) -> None:
"""Test that PEP8 error E262 (inline comment should start with '# ') is not triggered"""
"""Test that PEP8 error E262 (inline comment should start with '#') is not triggered"""
Copy link
Contributor

Choose a reason for hiding this comment

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

This change isn't correct (and also not related to the purpose of this PR). pycodestyle checks for a leading # and space character for comments.

CHANGELOG.md Outdated
@@ -45,6 +45,7 @@ list only contains the Pylint checkers enabled by default in PythonTA.
### 🔧 Internal changes

- Updated changelog and pull request template formats
- Added unit tests for pep8 errors E304, 303, 301, 275, 266 for `pycodestyle_checker`
Copy link
Contributor

Choose a reason for hiding this comment

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

Please follow the same entry as the wording in #1036

@lana-w
Copy link
Contributor Author

lana-w commented May 21, 2024

@david-yz-liu Made all requested changes!

david-yz-liu
david-yz-liu reviewed May 22, 2024
View reviewed changes
Copy link
Contributor

@david-yz-liu david-yz-liu left a comment

Choose a reason for hiding this comment

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

@lana-w I left one more comment. Additionally make sure you are running the pre-commit hooks on your computer (follow the setup instructions on the README); there shouldn't be any additional commits added to your PR by the pre-commit bot.

CHANGELOG.md Outdated
@@ -47,6 +47,8 @@ list only contains the Pylint checkers enabled by default in PythonTA.
- Updated changelog and pull request template formats
- Added unit tests for PEP8 errors E223, E224, E227, E228, E265 for `PycodestyleChecker`

Copy link
Contributor

Choose a reason for hiding this comment

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

Delete this blank line

lana-w and others added 6 commits May 22, 2024 11:25
@lana-w
Formatting clean-up
72a1127
@lana-w
A
6e8c72c 
Merge branch 'pycodestyle_checker-tests' of https://github.com/lana-w/pyta into pycodestyle_checker-tests

"Consolidating local and remote branches"
@lana-w
Revert "Formatting clean-up"
924ca8f 
This reverts commit 72a1127.
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
bff1863 
for more information, see https://pre-commit.ci
@lana-w
final formatting update
cb27cfb
@lana-w
Merge branch 'pycodestyle_checker-tests' of https://github.com/lana-w…
f37cc24 
…/pyta into pycodestyle_checker-tests

"
"Merging local to remote"
david-yz-liu
david-yz-liu reviewed May 24, 2024
View reviewed changes
CHANGELOG.md Outdated
@@ -46,6 +46,7 @@ list only contains the Pylint checkers enabled by default in PythonTA.

- Updated changelog and pull request template formats
- Added unit tests for PEP8 errors E223, E224, E227, E228, E265 for `PycodestyleChecker`
- Added unit tests for PEP8 errors E266, E275, E2301, E303, E304 for `PycodestyleChecker`
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a typo on this line, I think the code E2301 is incorrect.

@david-yz-liu david-yz-liu merged commit 92e5acd into pyta-uoft:master May 30, 2024
15 checks passed
@david-yz-liu david-yz-liu mentioned this pull request May 30, 2024
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@david-yz-liu david-yz-liu david-yz-liu 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.

None yet
3 participants
@lana-w @coveralls @david-yz-liu