-
Notifications
You must be signed in to change notification settings - Fork 3.8k
CASSANDRA-20021 Add cqlsh autocompletion for the identity mapping feature #4408
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
Changes from 2 commits
3819c51
8007fc3
0930359
55959b9
5a78cfd
ee5eeb5
c434d9a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -0,0 +1,71 @@ | ||||
# Licensed to the Apache Software Foundation (ASF) under one | ||||
# or more contributor license agreements. See the NOTICE file | ||||
# distributed with this work for additional information | ||||
# regarding copyright ownership. The ASF licenses this file | ||||
# to you under the Apache License, Version 2.0 (the | ||||
# "License"); you may not use this file except in compliance | ||||
# with the License. You may obtain a copy of the License at | ||||
# | ||||
# http://www.apache.org/licenses/LICENSE-2.0 | ||||
# | ||||
# Unless required by applicable law or agreed to in writing, software | ||||
# distributed under the License is distributed on an "AS IS" BASIS, | ||||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||
# See the License for the specific language governing permissions and | ||||
# limitations under the License. | ||||
|
||||
from .test_cqlsh_completion import CqlshCompletionCase | ||||
|
||||
|
||||
class TestIdentityMappingsCompletion(CqlshCompletionCase): | ||||
|
||||
""" | ||||
Autcomplete Tests for ADD and DROP Identity Mappings for @cql3handling.py | ||||
""" | ||||
|
||||
def test_identity_autocomplete_add(self): | ||||
self.trycompletions('ADD I', immediate='DENTITY ') | ||||
|
||||
def test_exists_autocomplete_add(self): | ||||
self.trycompletions('ADD IDENTITY IF ', immediate='NOT EXISTS ') | ||||
self.trycompletions('ADD IDENTITY IF NOT ', immediate='EXISTS ') | ||||
|
||||
def test_expect_str_literal_autocomplete_add(self): | ||||
self.trycompletions('ADD IDENTITY ', | ||||
choices=['<pgStringLiteral>', '<quotedStringLiteral>', 'IF']) | ||||
|
||||
|
||||
def test_TO_autocomplete_add(self): | ||||
self.trycompletions("ADD IDENTITY '[email protected]' ", immediate='TO ROLE ') | ||||
self.trycompletions("ADD IDENTITY '[email protected]' T", immediate='O ROLE ') | ||||
|
||||
def test_role_autocomplete_add(self): | ||||
self.trycompletions("ADD IDENTITY '[email protected]' TO ", immediate='ROLE ') | ||||
self.trycompletions("ADD IDENTITY '[email protected]' TO R", immediate='OLE ') | ||||
|
||||
def test_rolename_autocomplete_add(self): | ||||
self.trycompletions("ADD IDENTITY '[email protected]' TO ROLE ", | ||||
choices=['<identifier>', '<quotedName>'], | ||||
other_choices_ok=True) | ||||
|
||||
def test_complete_user_full_statement_add(self): | ||||
self.trycompletions("ADD IDENTITY IF NOT EXISTS '[email protected]' TO ROLE data_engineer ", | ||||
|
self.trycompletions("ADD IDENTITY '[email protected]' TO ROLE data_engineer ", |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hah! This is the same we are testing here
https://github.com/apache/cassandra/blob/trunk/pylib/cqlshlib/test/test_cqlsh_completion.py#L615-L618
I wonder then why that one is not failing. I think we should consolidate those.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test_complete_in_drop now expects IDENTITY and passes with it https://github.com/apache/cassandra/blob/09303597034af528c544bc9c1fb083c99cb33529/pylib/cqlshlib/test/test_cqlsh_completion.py#L613C5-L613C37
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I like the fact that we are splitting it into a new test file, I see that the pattern is to have them all consolidated in
test_cqlsh_completion.py
(that would have helped catching the DROP completion test duplication).I think we should add these tests there to follow the existing pattern, and maybe start a conversation on the dev slack (or Mailing list?) about splitting the completion tests (right now, a python file of 1242 lines and counting).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree here with Bernardo. We should probably try to consolidate the testing logic in
test_cqlsh_completion.py
. I think some existing autocompletion test scenarios will fail intest_cqlsh_completion.py
.I don't think we need a mailing list discussion for this change, I think it's pretty straightforward to make the change in
test_cqlsh_completion.py
, and we shouldn't really think too much about it.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK - identity mapping test file deleted in latest commit. Tests consolidated into test_cqlsh_completion.py. New tests are
TestCqlshCompletion:test_complete_in_add_identity
andTestCqlshCompletion: test_complete_in_drop_identity
. Fixes made to other existing tests in this file to account forIDENTITY
addition.