Fixed #2127 - WPS000 on several code fragments (Fragment 1) · Pull Request #2131 · wemake-services/wemake-python-styleguide

ghost · 2021-08-07T17:42:27Z

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

🙏 Please, if you or your company is finding wemake-python-styleguide valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/wemake-python-styleguide. As a thank you, your profile/company logo will be added to our main README which receives hundreds of unique visitors per day.

ghost · 2021-08-07T17:45:07Z

This is my first ever pull request, so please let me know if I'm doing something wrong. I identified the cause of the first fragment of issue #2127 and proposed a solution.

I tried creating a regression test, but not sure I did it correctly. Any advice or somewhere I can look at documentation would be appreciated!

sobolevn

You are doing great! Congrats on your first PR 🎉

Now CI will suggest some small nitpicks to fix before your PR can be merged, please, address the highlighted issues. And I will be happy to merge it!

sobolevn · 2021-08-07T18:07:26Z

You can also run flake8 . locally to find the problems.

ghost · 2021-08-07T18:49:30Z

Thanks @sobolevn! I appreciate it.

I think I fixed all the highlighted issues.

sobolevn

Awesome, thank you!

ghost · 2021-08-07T23:06:09Z

Thanks, so much @sobolevn. Is there anything else I need to do on my end for this issue?

sobolevn · 2021-08-08T08:41:09Z

Yes, there are three failing tests. We need to figure out what to do with them.

ghost

@sobolevn Whenever you have some time, could you take a look at this? I think this is part of why the errors are happening.

ghost · 2021-08-08T14:01:26Z

+
+
+def test_regression2127(
+    assert_errors,


It looks like the test is failing here. assert_errors is not a part of the pytest --fixtures list. But looking at other tests, it looks like that is used to assert whether any errors are occurring on a visitor. I tired to use the same method here, but it seems like it can't find assert_errors. Any idea what I'm missing here?

Yes, assert_errors is defined in tests/test_visitors.py: https://github.com/wemake-services/wemake-python-styleguide/blob/master/tests/test_visitors/conftest.py#L20

Move your tests into tests/test_visitors subfolder and it should be fine! 👍

ghost · 2021-08-11T21:58:39Z

@sobolevn Sorry to keep bothering you, but I can't seem to figure out what's going wrong with the test. I tried a few different things, but nothing seems to be working.

It looks like the issue is in the test_no_qa.py file. Any insight on where I should investigate as to why the tests are failing?

sobolevn · 2021-08-12T08:30:07Z

Hey! There are also style issues:

Raw Output:
./tests/test_visitors/test_if_else_one_line.py:2:1: I005 isort found an unexpected missing import
./tests/test_visitors/test_if_else_one_line.py:1:1: I001 isort found an import in the wrong position
./tests/test_visitors/test_if_else_one_line.py:1:81: E501 line too long (81 > 80 characters)
./tests/test_visitors/test_if_else_one_line.py:2:1: I005 isort found an unexpected missing import
./tests/test_visitors/test_if_else_one_line.py:2:1: I005 isort found an unexpected missing import
./tests/test_visitors/test_if_else_one_line.py:2:1: I005 isort found an unexpected missing import
=================================

./tests/test_visitors/test_if_else_one_line.py:1:1: F401 'wemake_python_styleguide.violations.refactoring.ImplicitElifViolation' imported but unused
./tests/test_visitors/test_if_else_one_line.py:1:1: I001 isort found an import in the wrong position
./tests/test_visitors/test_if_else_one_line.py:1:81: E501 line too long (81 > 80 characters)
./tests/test_visitors/test_if_else_one_line.py:2:1: I005 isort found an unexpected missing import
./tests/test_visitors/test_if_else_one_line.py:2:1: I005 isort found an unexpected missing import
./tests/test_visitors/test_if_else_one_line.py:2:1: I005 isort found an unexpected missing import
Process failed with the status code: 1

Let's fix them first, so we can actually see failing tests output.

ghost · 2021-08-12T12:52:06Z

Apologies. I fixed the style issues and ran flake8 and mypy locally with no issues, so hopefully that gets all of the style issues.

sobolevn · 2021-08-12T13:37:18Z

Looks like this error is not triggered: https://github.com/wemake-services/wemake-python-styleguide/blob/master/tests/fixtures/noqa/noqa.py#L614-L618

ghost · 2021-08-12T23:20:46Z

Thanks for that @sobolevn. That was super helpful in terms of helping me understand what was going on. I think I fixed the issue and passed all tests this time around.

codecov · 2021-08-21T10:12:13Z

Codecov Report

Merging #2131 (22ee7d8) into master (e70019e) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master     #2131   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          118       118           
  Lines         6207      6213    +6     
  Branches      1379      1379           
=========================================
+ Hits          6207      6213    +6

Impacted Files	Coverage Δ
..._python_styleguide/visitors/tokenize/conditions.py	`100.00% <100.00%> (ø)`
wemake_python_styleguide/logic/tree/keywords.py	`100.00% <0.00%> (ø)`
...emake_python_styleguide/visitors/ast/conditions.py	`100.00% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 46cbdd7...22ee7d8. Read the comment docs.

sobolevn · 2021-08-21T10:21:51Z

@danielboghean thanks again! 👍

Fixed #2127 - WPS000 on several code fragments (Fragment 1)

a1fbc55

sobolevn requested changes Aug 7, 2021

View reviewed changes

Fixed formatting issues

e1442e5

sobolevn approved these changes Aug 7, 2021

View reviewed changes

ghost commented Aug 9, 2021

View reviewed changes

Move test file to test_visitors (#2127) Fragment 1

2fd123b

Fixed formatting (#2127)

7c8d348

Fixed (#2127) Fragment #1

60acac9

Simplifies code

22ee7d8

sobolevn merged commit c7c77b7 into wemake-services:master Aug 21, 2021

Uh oh!

Conversation

ghost commented Aug 7, 2021 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

I have made things!

Checklist

Related issues

Uh oh!

ghost commented Aug 7, 2021

Uh oh!

sobolevn left a comment

Choose a reason for hiding this comment

Uh oh!

sobolevn commented Aug 7, 2021

Uh oh!

ghost commented Aug 7, 2021

Uh oh!

sobolevn left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Aug 7, 2021

Uh oh!

sobolevn commented Aug 8, 2021

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

ghost Aug 8, 2021

Choose a reason for hiding this comment

Uh oh!

sobolevn Aug 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Aug 11, 2021

Uh oh!

sobolevn commented Aug 12, 2021

Uh oh!

ghost commented Aug 12, 2021

Uh oh!

sobolevn commented Aug 12, 2021

Uh oh!

ghost commented Aug 12, 2021

Uh oh!

codecov Bot commented Aug 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sobolevn commented Aug 21, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ghost commented Aug 7, 2021 •

edited by ghost

Loading

sobolevn Aug 9, 2021 •

edited

Loading

codecov Bot commented Aug 21, 2021 •

edited

Loading