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

fix: Fixed E731 errors in tests #737

Closed
wants to merge 1 commit into from
Closed

fix: Fixed E731 errors in tests #737

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 33 additions & 35 deletions tests/test_format.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def test_strip_comments_preserves_linebreak(self):
assert res == 'select *\n\nfrom foo'

def test_strip_ws(self):
f = lambda sql: sqlparse.format(sql, strip_whitespace=True)
def f(sql): return sqlparse.format(sql, strip_whitespace=True)
Copy link
Owner

Choose a reason for hiding this comment

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

ah, I've got a different opinion on this. E731 states to prefer a function definition over assigning a lambda expression to a variable. But as also stated in E731 the primary reason is for debugging as the lambda expression shows up as <lambda> in a traceback.

But here the lambda is just a shorthand for the formatting code to make the tests shorter and more readable. Iff debugging is needed here, there's only one <lambda>. So it's pretty obvious.

On the other hand I find single line function definitions a bit harder to read and I would avoid them if possible.

So to me "Readability counts" is the argument here, especially as the one reason for using function definitions instead of assigning a lambda function to a variable doesn't count here.

Copy link
Author

Choose a reason for hiding this comment

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

Okay, I get you!)
I will close this PR now,
and thank you for your feedback

s = 'select\n* from foo\n\twhere ( 1 = 2 )\n'
assert f(s) == 'select * from foo where (1 = 2)'
s = 'select -- foo\nfrom bar\n'
Expand All @@ -115,7 +115,7 @@ def test_strip_ws_invalid_option(self):

def test_preserve_ws(self):
# preserve at least one whitespace after subgroups
f = lambda sql: sqlparse.format(sql, strip_whitespace=True)
def f(sql): return sqlparse.format(sql, strip_whitespace=True)
s = 'select\n* /* foo */ from bar '
assert f(s) == 'select * /* foo */ from bar'

Expand All @@ -128,7 +128,7 @@ def test_notransform_of_quoted_crlf(self):
s3 = "SELECT some_column LIKE 'value\\'\r' WHERE id = 1\r"
s4 = "SELECT some_column LIKE 'value\\\\\\'\r' WHERE id = 1\r\n"

f = lambda x: sqlparse.format(x)
def f(x): return sqlparse.format(x)

# Because of the use of
assert f(s1) == "SELECT some_column LIKE 'value\r'"
Expand All @@ -140,8 +140,7 @@ def test_notransform_of_quoted_crlf(self):

class TestFormatReindentAligned:
@staticmethod
def formatter(sql):
return sqlparse.format(sql, reindent_aligned=True)
def formatter(sql): return sqlparse.format(sql, reindent_aligned=True)
Copy link
Owner

Choose a reason for hiding this comment

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

Why did you change this to a single line expression? It's harder to read, IMO.


def test_basic(self):
sql = """
Expand Down Expand Up @@ -354,7 +353,7 @@ def test_option(self):
sqlparse.format('foo', reindent=True, comma_first='foo')

def test_stmts(self):
f = lambda sql: sqlparse.format(sql, reindent=True)
def f(sql): return sqlparse.format(sql, reindent=True)
s = 'select foo; select bar'
assert f(s) == 'select foo;\n\nselect bar'
s = 'select foo'
Expand All @@ -363,7 +362,7 @@ def test_stmts(self):
assert f(s) == 'select foo; -- test\n\nselect bar'

def test_keywords(self):
f = lambda sql: sqlparse.format(sql, reindent=True)
def f(sql): return sqlparse.format(sql, reindent=True)
s = 'select * from foo union select * from bar;'
assert f(s) == '\n'.join([
'select *',
Expand All @@ -375,15 +374,15 @@ def test_keywords(self):
def test_keywords_between(self):
# issue 14
# don't break AND after BETWEEN
f = lambda sql: sqlparse.format(sql, reindent=True)
def f(sql): return sqlparse.format(sql, reindent=True)
s = 'and foo between 1 and 2 and bar = 3'
assert f(s) == '\n'.join([
'',
'and foo between 1 and 2',
'and bar = 3'])

def test_parenthesis(self):
f = lambda sql: sqlparse.format(sql, reindent=True)
def f(sql): return sqlparse.format(sql, reindent=True)
s = 'select count(*) from (select * from foo);'
assert f(s) == '\n'.join([
'select count(*)',
Expand All @@ -397,7 +396,7 @@ def test_parenthesis(self):
assert f("select f(\n\n\n 1 \n\n\n)") == 'select f(1)'

def test_where(self):
f = lambda sql: sqlparse.format(sql, reindent=True)
def f(sql): return sqlparse.format(sql, reindent=True)
s = 'select * from foo where bar = 1 and baz = 2 or bzz = 3;'
assert f(s) == '\n'.join([
'select *',
Expand All @@ -415,7 +414,7 @@ def test_where(self):
' or bzz = 3);'])

def test_join(self):
f = lambda sql: sqlparse.format(sql, reindent=True)
def f(sql): return sqlparse.format(sql, reindent=True)
s = 'select * from foo join bar on 1 = 2'
assert f(s) == '\n'.join([
'select *',
Expand All @@ -438,7 +437,7 @@ def test_join(self):
'straight_join bar on 1 = 2'])

def test_identifier_list(self):
f = lambda sql: sqlparse.format(sql, reindent=True)
def f(sql): return sqlparse.format(sql, reindent=True)
s = 'select foo, bar, baz from table1, table2 where 1 = 2'
assert f(s) == '\n'.join([
'select foo,',
Expand All @@ -455,7 +454,7 @@ def test_identifier_list(self):
' b'])

def test_identifier_list_with_wrap_after(self):
f = lambda sql: sqlparse.format(sql, reindent=True, wrap_after=14)
def f(sql): return sqlparse.format(sql, reindent=True, wrap_after=14)
s = 'select foo, bar, baz from table1, table2 where 1 = 2'
assert f(s) == '\n'.join([
'select foo, bar,',
Expand All @@ -464,7 +463,7 @@ def test_identifier_list_with_wrap_after(self):
'where 1 = 2'])

def test_identifier_list_comment_first(self):
f = lambda sql: sqlparse.format(sql, reindent=True, comma_first=True)
def f(sql): return sqlparse.format(sql, reindent=True, comma_first=True)
# not the 3: It cleans up whitespace too!
s = 'select foo, bar, baz from table where foo in (1, 2,3)'
assert f(s) == '\n'.join([
Expand All @@ -477,7 +476,7 @@ def test_identifier_list_comment_first(self):
' , 3)'])

def test_identifier_list_with_functions(self):
f = lambda sql: sqlparse.format(sql, reindent=True)
def f(sql): return sqlparse.format(sql, reindent=True)
s = ("select 'abc' as foo, coalesce(col1, col2)||col3 as bar,"
"col3 from my_table")
assert f(s) == '\n'.join([
Expand All @@ -487,7 +486,7 @@ def test_identifier_list_with_functions(self):
"from my_table"])

def test_long_identifier_list_with_functions(self):
f = lambda sql: sqlparse.format(sql, reindent=True, wrap_after=30)
def f(sql): return sqlparse.format(sql, reindent=True, wrap_after=30)
s = ("select 'abc' as foo, json_build_object('a', a,"
"'b', b, 'c', c, 'd', d, 'e', e) as col2"
"col3 from my_table")
Expand All @@ -499,7 +498,7 @@ def test_long_identifier_list_with_functions(self):
"from my_table"])

def test_case(self):
f = lambda sql: sqlparse.format(sql, reindent=True)
def f(sql): return sqlparse.format(sql, reindent=True)
s = 'case when foo = 1 then 2 when foo = 3 then 4 else 5 end'
assert f(s) == '\n'.join([
'case',
Expand All @@ -509,7 +508,7 @@ def test_case(self):
'end'])

def test_case2(self):
f = lambda sql: sqlparse.format(sql, reindent=True)
def f(sql): return sqlparse.format(sql, reindent=True)
s = 'case(foo) when bar = 1 then 2 else 3 end'
assert f(s) == '\n'.join([
'case(foo)',
Expand All @@ -519,7 +518,7 @@ def test_case2(self):

def test_nested_identifier_list(self):
# issue4
f = lambda sql: sqlparse.format(sql, reindent=True)
def f(sql): return sqlparse.format(sql, reindent=True)
s = '(foo as bar, bar1, bar2 as bar3, b4 as b5)'
assert f(s) == '\n'.join([
'(foo as bar,',
Expand All @@ -529,7 +528,7 @@ def test_nested_identifier_list(self):

def test_duplicate_linebreaks(self):
# issue3
f = lambda sql: sqlparse.format(sql, reindent=True)
def f(sql): return sqlparse.format(sql, reindent=True)
s = 'select c1 -- column1\nfrom foo'
assert f(s) == '\n'.join([
'select c1 -- column1',
Expand All @@ -553,7 +552,7 @@ def test_duplicate_linebreaks(self):

def test_keywordfunctions(self):
# issue36
f = lambda sql: sqlparse.format(sql, reindent=True)
def f(sql): return sqlparse.format(sql, reindent=True)
s = 'select max(a) b, foo, bar'
assert f(s) == '\n'.join([
'select max(a) b,',
Expand All @@ -562,7 +561,7 @@ def test_keywordfunctions(self):

def test_identifier_and_functions(self):
# issue45
f = lambda sql: sqlparse.format(sql, reindent=True)
def f(sql): return sqlparse.format(sql, reindent=True)
s = 'select foo.bar, nvl(1) from dual'
assert f(s) == '\n'.join([
'select foo.bar,',
Expand All @@ -571,7 +570,7 @@ def test_identifier_and_functions(self):

def test_insert_values(self):
# issue 329
f = lambda sql: sqlparse.format(sql, reindent=True)
def f(sql): return sqlparse.format(sql, reindent=True)
s = 'insert into foo values (1, 2)'
assert f(s) == '\n'.join([
'insert into foo',
Expand All @@ -591,8 +590,7 @@ def test_insert_values(self):
' (3, 4),',
' (5, 6)'])

f = lambda sql: sqlparse.format(sql, reindent=True,
comma_first=True)
def f(sql): return sqlparse.format(sql, reindent=True, comma_first=True)
s = 'insert into foo values (1, 2)'
assert f(s) == '\n'.join([
'insert into foo',
Expand All @@ -616,26 +614,27 @@ def test_insert_values(self):
class TestOutputFormat:
def test_python(self):
sql = 'select * from foo;'
f = lambda sql: sqlparse.format(sql, output_format='python')
def f(sql): return sqlparse.format(sql, output_format='python')
assert f(sql) == "sql = 'select * from foo;'"
f = lambda sql: sqlparse.format(sql, output_format='python',
reindent=True)
def f(sql): return sqlparse.format(sql, output_format='python', reindent=True)
Copy link
Owner

Choose a reason for hiding this comment

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

This and some of the following changes break the 80-column rule.

assert f(sql) == '\n'.join([
"sql = ('select * '",
" 'from foo;')"])

def test_python_multiple_statements(self):
sql = 'select * from foo; select 1 from dual'
f = lambda sql: sqlparse.format(sql, output_format='python')
def f(sql):
return sqlparse.format(sql, output_format='python')
assert f(sql) == '\n'.join([
"sql = 'select * from foo; '",
"sql2 = 'select 1 from dual'"])

@pytest.mark.xfail(reason="Needs fixing")
def test_python_multiple_statements_with_formatting(self):
sql = 'select * from foo; select 1 from dual'
f = lambda sql: sqlparse.format(sql, output_format='python',
reindent=True)
def f(sql): return sqlparse.format(sql, output_format='python', reindent=True)
from pprint import pprint
Copy link
Owner

Choose a reason for hiding this comment

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

Looks like you've left some debugging code in this change.

pprint(f(sql))
assert f(sql) == '\n'.join([
"sql = ('select * '",
" 'from foo;')",
Expand All @@ -644,18 +643,17 @@ def test_python_multiple_statements_with_formatting(self):

def test_php(self):
sql = 'select * from foo;'
f = lambda sql: sqlparse.format(sql, output_format='php')
def f(sql): return sqlparse.format(sql, output_format='php')
assert f(sql) == '$sql = "select * from foo;";'
f = lambda sql: sqlparse.format(sql, output_format='php',
reindent=True)
def f(sql): return sqlparse.format(sql, output_format='php', reindent=True)
assert f(sql) == '\n'.join([
'$sql = "select * ";',
'$sql .= "from foo;";'])

def test_sql(self):
# "sql" is an allowed option but has no effect
sql = 'select * from foo;'
f = lambda sql: sqlparse.format(sql, output_format='sql')
def f(sql): return sqlparse.format(sql, output_format='sql')
assert f(sql) == 'select * from foo;'

def test_invalid_option(self):
Expand Down