Skip to content

Add more tests for parseutils.py#1758

Merged
rolandwalker merged 1 commit intomainfrom
RW/add-parseutils-tests
Mar 31, 2026
Merged

Add more tests for parseutils.py#1758
rolandwalker merged 1 commit intomainfrom
RW/add-parseutils-tests

Conversation

@rolandwalker
Copy link
Copy Markdown
Contributor

Description

Add more tests for parseutils.py, covering SQL parsing, though there is some non-SQL as well. Added tests for

  • extract_from_part()
  • extract_table_identifiers()
  • find_prev_keyword()
  • get_last_select()
  • is_subselect()
  • is_valid_connection_scheme()
  • last_word()
  • query_is_single_table_update()

Also:

  • adding the tests inspired catching a possible IndexError in query_is_single_table_update().
  • removed an unused __main__ section from parseutils.py
  • accidentally changed some quoting in the tests
  • test_extract_columns_from_select() seemed to incorrectly catch an exception, which was removed. If tests can raise, that is something that should itself be tested, not covered up.

I couldn't figure out how to test the punctuation parameter to extract_from_part() and left a todo comment.

This is covered by an existing changelog entry.

Checklist

  • I added this contribution to the changelog.md file.
  • I added my name to the AUTHORS file (or it's already there).
  • To lint and format the code, I ran
    uv run ruff check && uv run ruff format && uv run mypy --install-types .

covering SQL parsing, though there is some non-SQL as well.  Added
tests for

 * extract_from_part()
 * extract_table_identifiers()
 * find_prev_keyword()
 * get_last_select()
 * is_subselect()
 * is_valid_connection_scheme()
 * last_word()
 * query_is_single_table_update()

Also:
 * adding the tests inspired catching a possible IndexError in
query_is_single_table_update().
 * removed an unused __main__ section from parseutils.py
 * accidentally changed some quoting in the tests
 * test_extract_columns_from_select() seemed to incorrectly catch an
   exception, which was removed.  If tests can raise, that is something
   that should itself be tested, not covered up.
@rolandwalker rolandwalker self-assigned this Mar 31, 2026
@github-actions
Copy link
Copy Markdown

Findings

  1. Low: Missing regression test for the actual IndexError path this patch fixes.
    The new guard in query_is_single_table_update (try/except IndexError at line 382) is good, but the new parametrized test in test_parseutils.py does not cover malformed UPDATE ... WHERE ... statements that previously could trigger that path via is_destructive().
    Action: add a case like update test where id = 1 (and optionally is_destructive(["update"], "...UPDATE test WHERE id = 1;")) to lock in the fix.

No correctness/security regressions stood out in the diff beyond that testing gap.

Could not run tests in this environment because uv/pytest dependencies are unavailable.

@rolandwalker rolandwalker merged commit 96771c4 into main Mar 31, 2026
10 checks passed
@rolandwalker rolandwalker deleted the RW/add-parseutils-tests branch March 31, 2026 16:16
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.

2 participants