Skip to content

Give toolbar tests the sqlexecute property#1722

Open
rolandwalker wants to merge 1 commit intomainfrom
RW/give-toolbar-tests-db-connections
Open

Give toolbar tests the sqlexecute property#1722
rolandwalker wants to merge 1 commit intomainfrom
RW/give-toolbar-tests-db-connections

Conversation

@rolandwalker
Copy link
Contributor

Description

Now that the prompt string can have data fetched from the connection, and now that the toolbar can have custom format strings, there are assertions in get_prompt() that we should satisfy in toolbar tests, by setting the sqlexecute property.

It's not clear how this has been passing up until now in CI.

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 .
    

@rolandwalker rolandwalker self-assigned this Mar 17, 2026
@github-actions
Copy link

Findings:

  1. Medium: these tests were converted from unit tests into integration tests, so they now get skipped when no DB is available (@dbtest), which removes toolbar coverage in many environments and can hide regressions in non-DB toolbar behavior.

  2. Low: each test opens a real DB connection via SQLExecute(...) and never closes it. This can leak connections in longer test runs.

Security concerns:

  • No new security issues identified in this PR’s diff.

Validation note:

  • I could not execute tests in this environment because pytest/uv are not installed.

Now that the prompt string can have data fetched from the connection,
and now that the toolbar can have custom format strings, there are
assertions in get_prompt() that we should satisfy in toolbar tests, by
setting the sqlexecute property.
@rolandwalker rolandwalker force-pushed the RW/give-toolbar-tests-db-connections branch from 0a64c3e to 86c1f54 Compare March 17, 2026 09:32
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.

1 participant