Skip to content

test(redis): cover a backslash in list_sessions glob escaping - #7216

Open
vishal-bulbule wants to merge 1 commit into
google:mainfrom
vishal-bulbule:test/redis-backslash-glob
Open

vishal-bulbule wants to merge 1 commit into
google:mainfrom
vishal-bulbule:test/redis-backslash-glob

Conversation

@vishal-bulbule

Copy link
Copy Markdown

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

  • N/A

2. Or, if no issue exists, describe the change:

This is the follow-up test a maintainer asked for in
#6956:
"the backslash case still isn't covered, so that test is worth sending on its own."

Problem:

4b26bba made RedisSessionService.list_sessions match app_name and user_id literally by
escaping Redis glob metacharacters. _escape_glob escapes the backslash along with *, ?,
[ and ], but test_list_sessions_glob_metacharacters_match_literally had no backslash
case. User IDs such as CORP\alice depend on it.

Solution:

Add a backslash case to the existing test's table in
tests/unittests/integrations/redis/test_redis_session_service.py. Test-only change.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

The new case passes on main, and fails if the backslash is removed from _escape_glob
(checked by temporarily editing the regex):

$ pytest tests/unittests/integrations/redis -k glob_metacharacters
1 passed
(with the backslash removed from _escape_glob: 1 failed)
$ pytest tests/unittests -n auto
15287 passed, 1 failed

$ tox   (Python 3.10, 3.11, 3.12, 3.13, 3.14)
py310: 15221 passed, 1 failed
py311: 15230 passed, 1 failed
py312: 15221 passed, 1 failed
py313: 15221 passed, 1 failed
py314: 15221 passed, 1 failed

The one failing test below, test_import_loading.py::test_entry_point_loads_only_allowlisted_packages[agent],
also fails on main (d57c84f) and is unrelated to this change; #7214 fixes it.

Manual End-to-End (E2E) Tests:

N/A, test-only change.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas. (N/A, test-only)
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end. (N/A, test-only)
  • Any dependent changes have been merged and published in downstream modules. (N/A)

_escape_glob escapes the backslash along with *, ?, [ and ], but the test
that list_sessions matches app and user IDs literally had no backslash
case. Add one, which also covers user IDs such as CORP\alice.
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