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

Add test for "1 / any number" expression #786

Merged
merged 3 commits into from
Aug 25, 2019

Conversation

serhii73
Copy link
Contributor

Fix #775

@serhii73
Copy link
Contributor Author

@sobolevn is it a right test for #775?

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.

Yes, it is! Thanks for asking!

@serhii73
Copy link
Contributor Author

Screenshot from 2019-08-23 18-53-57
But why I get the error in tests/test_visitors/test_ast/conftest.py ?

@serhii73
Copy link
Contributor Author

@serhii73
Copy link
Contributor Author

I added ipdb to code and want to understand where code return error and where need fix function.

@pytest.mark.parametrize('expression', [
    '1 / other',
    '1 / 11',
])
def test_one_to_divide(
    assert_errors,
    parse_ast_tree,
    expression,
    default_options,
):
    """Testing that `1 / any number` is the correct expression."""

    tree = parse_ast_tree(usage_template.format(expression))
    import ipdb; ipdb.set_trace()

    visitor = UselessOperatorsVisitor(default_options, tree=tree)
    visitor.run()

    assert_errors(visitor, [])

Because of the need to fix this problem - #775
But I see Syntax Error.

@sobolevn
Copy link
Member

Try tree = parse_ast_tree(expression)

@pytest.mark.parametrize('expression', [
'1 / other',
'1 / 11',
'1 / 1.1',
Copy link
Member

Choose a reason for hiding this comment

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

Please, also check 1 / 1. It should raise a violation because of the / 1 part

@sobolevn
Copy link
Member

@serhii73 thanks! I will take it from here, since I am releasing 0.12.3 at the moment.

@sobolevn sobolevn merged commit cd14300 into wemake-services:master Aug 25, 2019
@serhii73
Copy link
Contributor Author

You're welcome.
Sorry but I don't understand why Travis is red, but I'm very curious. Do you know? @sobolevn

@sobolevn
Copy link
Member

See my fix: 9b3c155#diff-b9425bbc8c1b7baf877d6cbb75055e64R109

It was a mypy issue.

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

Successfully merging this pull request may close these issues.

False positive for WPS345 Found meaningless number operation
2 participants