Skip to content

Commit

Permalink
Improved error messages for CLI commands and fixed shell testing issu…
Browse files Browse the repository at this point in the history
…es (#2953)

I have read and agree to the CLA of the Kuzu repository.
  • Loading branch information
MSebanc committed Feb 26, 2024
1 parent d2cfc65 commit f12ae4c
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 2,496 deletions.
54 changes: 51 additions & 3 deletions tools/shell/embedded_shell.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,50 @@ void highlight(char* buffer, char* resultBuf, uint32_t renderWidth, uint32_t cur
}
// Linenoise allocates a fixed size buffer for the current line's contents, and doesn't export
// the length.
constexpr size_t LINENOISE_MAX_LINE = 4096;
constexpr uint64_t LINENOISE_MAX_LINE = 4096;
strncpy(resultBuf, highlightBuffer.str().c_str(), LINENOISE_MAX_LINE - 1);
}

uint64_t damerauLevenshteinDistance(const std::string& s1, const std::string& s2) {
const uint64_t m = s1.size(), n = s2.size();
std::vector<std::vector<uint64_t>> dp(m + 1, std::vector<uint64_t>(n + 1, 0));
for (uint64_t i = 0; i <= m; i++) {
dp[i][0] = i;
}
for (uint64_t j = 0; j <= n; j++) {
dp[0][j] = j;
}
for (uint64_t i = 1; i <= m; i++) {
for (uint64_t j = 1; j <= n; j++) {
if (s1[i - 1] == s2[j - 1]) {
dp[i][j] = dp[i - 1][j - 1];
if (i > 1 && j > 1 && s1[i - 1] == s2[j - 2] && s1[i - 2] == s2[j - 1]) {
dp[i][j] = std::min(dp[i][j], dp[i - 2][j - 2]);
}
} else {
dp[i][j] = 1 + std::min({dp[i - 1][j], dp[i][j - 1], dp[i - 1][j - 1]});
if (i > 1 && j > 1 && s1[i - 1] == s2[j - 2] && s1[i - 2] == s2[j - 1]) {
dp[i][j] = std::min(dp[i][j], dp[i - 2][j - 2] + 1);
}
}
}
}
return dp[m][n];
}

std::string findClosestCommand(std::string lineStr) {
std::string closestCommand = "";
uint64_t editDistance = INT_MAX;
for (auto& command : shellCommand.commandList) {
auto distance = damerauLevenshteinDistance(command, lineStr);
if (distance < editDistance) {
editDistance = distance;
closestCommand = command;
}
}
return closestCommand;
}

int EmbeddedShell::processShellCommands(std::string lineStr) {
if (lineStr == shellCommand.HELP) {
printHelp();
Expand All @@ -239,6 +279,7 @@ int EmbeddedShell::processShellCommands(std::string lineStr) {
setMaxWidth(lineStr.substr(strlen(shellCommand.MAXWIDTH)));
} else {
printf("Error: Unknown command: \"%s\". Enter \":help\" for help\n", lineStr.c_str());
printf("Did you mean: \"%s\"?\n", findClosestCommand(lineStr).c_str());
}
return 0;
}
Expand Down Expand Up @@ -316,7 +357,14 @@ void EmbeddedShell::run() {
if (queryResult->isSuccess()) {
printExecutionResult(*queryResult);
} else {
printf("Error: %s\n", queryResult->getErrorMessage().c_str());
std::string lineStrTrimmed = lineStr;
lineStrTrimmed = lineStrTrimmed.erase(0, lineStr.find_first_not_of(" \t\n\r\f\v"));
if (lineStrTrimmed.find_first_of(" \t\n\r\f\v") == std::string::npos && lineStrTrimmed.length() > 1) {
printf("Error: \"%s\" is not a valid Cypher query. Did you mean to issue a CLI command, e.g., \"%s\"?\n", lineStr.c_str(), findClosestCommand(lineStrTrimmed).c_str());
}
else {
printf("Error: %s\n", queryResult->getErrorMessage().c_str());
}
}
}
} else if (!lineStr.empty() && lineStr[0] != ':') {
Expand Down Expand Up @@ -587,7 +635,7 @@ void EmbeddedShell::printExecutionResult(QueryResult& queryResult) const {
}
auto tuple = queryResult.getNext();
auto result = tuple->toString(colsWidth, "|", maxWidth);
size_t startPos = 0;
uint64_t startPos = 0;
std::vector<std::string> colResults;
for (auto i = 0u; i < colsWidth.size(); i++) {
uint32_t chrIter = startPos;
Expand Down
4 changes: 2 additions & 2 deletions tools/shell/test/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import shutil
import subprocess
import pexpect
from test_helper import KUZU_EXEC_PATH, KUZU_ROOT
from test_helper import *
from typing import List, Union


Expand Down Expand Up @@ -167,7 +167,7 @@ def get_tmp_path(tmp_path):
@pytest.fixture
def history_path():
path = os.path.join(KUZU_ROOT, 'tools', 'shell', 'test', 'files')
shutil.rmtree(os.path.join(path, "history.txt"), ignore_errors=True)
deleteIfExists(os.path.join(path, 'history.txt'))
return path


Expand Down
2,477 changes: 0 additions & 2,477 deletions tools/shell/test/files/vPerson.csv

Large diffs are not rendered by default.

6 changes: 5 additions & 1 deletion tools/shell/test/test_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,8 @@ class KEY_ACTION(Enum):
CTRL_W = chr(23) # Ctrl-w
ESC = '\27' # Escape
BACKSPACE = chr(127) # Backspace



def deleteIfExists(file):
if os.path.exists(file):
os.remove(file)
11 changes: 6 additions & 5 deletions tools/shell/test/test_shell_basics.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import pytest
import shutil
from test_helper import *
from conftest import ShellTest

Expand Down Expand Up @@ -87,7 +86,8 @@ def test_multi_queries_one_line(temp_db):
)
result = test.run()
result.check_stdout("databases rule")
result.check_stdout(["Error: Parser exception: Invalid input < >: expected rule oC_Cypher (line: 1, offset: 6)"])
result.check_stdout(["Error: Parser exception: mismatched input '<EOF>' expecting {CALL, COMMENT, COPY, EXPORT, DROP, ALTER, BEGIN, COMMIT, COMMIT_SKIP_CHECKPOINT, ROLLBACK, ROLLBACK_SKIP_CHECKPOINT, INSTALL, LOAD, OPTIONAL, MATCH, UNWIND, CREATE, MERGE, SET, DETACH, DELETE, WITH, RETURN} (line: 1, offset: 6)",
'" "'])

# two failing queries
test = (
Expand All @@ -96,10 +96,11 @@ def test_multi_queries_one_line(temp_db):
.statement('RETURN "databases rule" S a; ;')
)
result = test.run()
result.check_stdout(["Error: Parser exception: Invalid input < S>: expected rule oC_Cypher (line: 1, offset: 24)",
result.check_stdout(["Error: Parser exception: Invalid input < S>: expected rule ku_Statements (line: 1, offset: 24)",
'"RETURN "databases rule" S a"',
" ^",
"Error: Parser exception: Invalid input < >: expected rule oC_Cypher (line: 1, offset: 6)"])
"Error: Parser exception: mismatched input '<EOF>' expecting {CALL, COMMENT, COPY, EXPORT, DROP, ALTER, BEGIN, COMMIT, COMMIT_SKIP_CHECKPOINT, ROLLBACK, ROLLBACK_SKIP_CHECKPOINT, INSTALL, LOAD, OPTIONAL, MATCH, UNWIND, CREATE, MERGE, SET, DETACH, DELETE, WITH, RETURN} (line: 1, offset: 6)",
'" "'])


def test_row_truncation(temp_db, csv_path):
Expand Down Expand Up @@ -155,5 +156,5 @@ def test_history_consecutive_repeats(temp_db, history_path):
assert f.readline() == ''
f.close()

shutil.rmtree(os.path.join(history_path, "history.txt"), ignore_errors=True)
deleteIfExists(os.path.join(history_path, 'history.txt'))

17 changes: 16 additions & 1 deletion tools/shell/test/test_shell_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,4 +132,19 @@ def test_max_width(temp_db, csv_path):
result.check_stdout("Node table: LANGUAGE_CODE has been created.")
result.check_not_stdout("| ... |")
result.check_stdout("(1 column)")



def test_bad_command(temp_db):
test = (
ShellTest()
.add_argument(temp_db)
.statement(":maxrows")
.statement(":quiy")
.statement("clearr;")
)
result = test.run()
result.check_stdout('Error: Unknown command: ":maxrows". Enter ":help" for help')
result.check_stdout('Did you mean: ":max_rows"?')
result.check_stdout('Error: Unknown command: ":quiy". Enter ":help" for help')
result.check_stdout('Did you mean: ":quit"?')
result.check_stdout('Error: "clearr;" is not a valid Cypher query. Did you mean to issue a CLI command, e.g., ":clear"?')
5 changes: 2 additions & 3 deletions tools/shell/test/test_shell_control_edit.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import pytest
import pexpect
import shutil
from test_helper import *
from conftest import ShellTest

Expand Down Expand Up @@ -61,7 +60,7 @@ def test_enter(temp_db, key, history_path):
f = open(os.path.join(history_path, "history.txt"))
assert f.readline() == 'RETURN "databases rule" AS a;\n'
f.close()
shutil.rmtree(os.path.join(history_path, "history.txt"), ignore_errors=True)
deleteIfExists(os.path.join(history_path, 'history.txt'))


@pytest.mark.parametrize(
Expand Down Expand Up @@ -198,7 +197,7 @@ def test_search(temp_db, key, history_path):
# enter should process last match
test.send_finished_statement(KEY_ACTION.ENTER.value)
assert test.shell_process.expect_exact(["| databases rule |", pexpect.EOF]) == 0
shutil.rmtree(os.path.join(history_path, "history.txt"), ignore_errors=True)
deleteIfExists(os.path.join(history_path, 'history.txt'))

# test starting search with text inputted already
test = ShellTest().add_argument(temp_db)
Expand Down
3 changes: 1 addition & 2 deletions tools/shell/test/test_shell_control_search.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import pytest
import pexpect
import shutil
from test_helper import *
from conftest import ShellTest

Expand Down Expand Up @@ -60,7 +59,7 @@ def test_search_next(temp_db, key, history_path):
test.send_control_statement(key)
test.send_finished_statement(KEY_ACTION.ENTER.value)
assert test.shell_process.expect_exact(["| databases rule |", pexpect.EOF]) == 0
shutil.rmtree(os.path.join(history_path, "history.txt"), ignore_errors=True)
deleteIfExists(os.path.join(history_path, 'history.txt'))


@pytest.mark.parametrize(
Expand Down
3 changes: 1 addition & 2 deletions tools/shell/test/test_shell_flags.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import pytest
import shutil
from test_helper import *
from conftest import ShellTest
import os
Expand Down Expand Up @@ -178,7 +177,7 @@ def test_history_path(temp_db, history_path):
assert f.readline() == 'RETURN "kuzu is cool" AS b;\n'
f.close()

shutil.rmtree(os.path.join(history_path, "history.txt"), ignore_errors=True)
deleteIfExists(os.path.join(history_path, 'history.txt'))


@pytest.mark.parametrize(
Expand Down

0 comments on commit f12ae4c

Please sign in to comment.