dbcli / mycli

A Terminal Client for MySQL with AutoCompletion and Syntax Highlighting.
http://mycli.net
Other
11.49k stars 663 forks source link

test_auto_escaped_col_names fails with sqlparse 0.4.3 #1103

Closed jfly closed 7 months ago

jfly commented 1 year ago

test/test_smart_completion_public_schema_only.py::test_auto_escaped_col_names fails when run under sqlparse 0.4.3, but passes when run under sqlparse 0.4.2:

Passes with sqlparse 0.4.2:

$ git checkout v1.26.1 && pip install -r requirements-dev.txt && pip install -e . && pip install 'sqlparse==0.4.2' && pytest test/test_smart_completion_public_schema_only.py::test_auto_escaped_col_names
test/test_smart_completion_public_schema_only.py .                                                                                                      [100%]

====================================================================== 1 passed in 0.01s ======================================================================

Fails with sqlparse 0.4.3:

$ git checkout v1.26.1 && pip install -r requirements-dev.txt && pip install -e . && pip install 'sqlparse==0.4.3' && pytest test/test_smart_completion_public_schema_only.py::test_auto_escaped_col_names
...
test/test_smart_completion_public_schema_only.py F                                                                                                      [100%]

========================================================================== FAILURES ===========================================================================
_________________________________________________________________ test_auto_escaped_col_names _________________________________________________________________

completer = <mycli.sqlcompleter.SQLCompleter object at 0x7fb732a8e1f0>, complete_event = <Mock id='140424805671888'>

    def test_auto_escaped_col_names(completer, complete_event):
        text = 'SELECT  from `select`'
        position = len('SELECT ')
        result = list(completer.get_completions(
            Document(text=text, cursor_position=position),
            complete_event))
>       assert result == [
            Completion(text='*', start_position=0),
            Completion(text='`ABC`', start_position=0),
            Completion(text='`insert`', start_position=0),
            Completion(text='id', start_position=0),
        ] + \
            list(map(Completion, completer.functions)) + \
            [Completion(text='`select`', start_position=0)] + \
            list(map(Completion, completer.keywords))
E       AssertionError: assert [Completion(t...CAT')])), ...] == [Completion(t...CAT')])), ...]
E         At index 23 diff: Completion(text='select', start_position=0, display=FormattedText([('', 'select')])) != Completion(text='`select`', start_position=0, display=FormattedText([('', '`select`')]))
E         Use -v to get more diff

test/test_smart_completion_public_schema_only.py:318: AssertionError
=================================================================== short test summary info ===================================================================
FAILED test/test_smart_completion_public_schema_only.py::test_auto_escaped_col_names - AssertionError: assert [Completion(t...CAT')])), ...] == [Completion(t...CAT')])), ...]
====================================================================== 1 failed in 0.06s ======================================================================

I dug a bit and this seems to be happening because of this commit https://github.com/andialbrecht/sqlparse/commit/c1a597ee6e8e22c94ca5bdfa1c6665453e0d2c25 that went out in sqlparse 0.4.3.

I'm not entirely sure if this is revealing a bug in mycli (should it be escaping table names like this if necessary?) or if this is a perfectly fine change in behavior, and the test should just be updated to allow the new behavior.

parona-source commented 7 months ago

This should be addressed at least indirectly by https://github.com/dbcli/mycli/commit/f3f50c81c14a6091057b2234af5d3527a48d1dfa

jfly commented 7 months ago

Yep, that fixed it! I'll re-enable this test back in nixos: https://github.com/NixOS/nixpkgs/pull/305233, but I think this issue can get closed.