Add unit tests for our authentication flows.

Review Request #14598 — Created Sept. 8, 2025 and updated

Information

RBTools
release-5.x

Reviewers

This adds unit tests for our authentication flows (our HTTP auth handlers
andvsession authentication). These flows have recently been updated to
include web-based login, so it's a good time to add unit tests for
these.

There's a TODO note left in here to flesh out the OTP login flow, since
this is too outside of the scope of my web-login changes.

Ran unit tests.

Summary ID
Add unit tests for our authentication flows.
This adds unit tests for our authentication flows (our HTTP auth handlers andvsession authentication). These flows have recently been updated to include web-based login, so it's a good time to add unit tests for these. There's a TODO note left in here to flesh out the OTP login flow, since this is too outside of the scope of my web-login changes.
3cf2f8915595907a3bf5147ec4809cd39bb5574e
Description From Last Updated

You can use b:'user:pass' instead of encoding. Here and below.

chipx86chipx86

Minor nit, but we generally put owner= right after the spy and before the op, so it's better tied to …

chipx86chipx86

This could use a Version Added.

chipx86chipx86

I believe this needs an owner=.

chipx86chipx86

Same here.

chipx86chipx86

Same here.

chipx86chipx86

Same here. And the ones below.

chipx86chipx86

This should be in multi-line form.

chipx86chipx86

This also needs owner=.

chipx86chipx86

Can we call this _TestCommand? Often having classes with the name Test* will generate warnings about collecting test cases.

daviddavid

Test docstrings should all start with "Testing" This applies throughout this entire change.

daviddavid

This needs a docstring.

daviddavid

Typo: toke -> token

daviddavid

Instead of "Hit" perhaps we can use "Get"?

daviddavid

This should use (list[str] | None) = None

daviddavid

Actually, just one note. This can be: args=[*command_args, '--debug']

daviddavid
chipx86
  1. 
      
  2. rbtools/api/tests/test_auth_handlers.py (Diff revision 1)
     
     
    Show all issues

    You can use b:'user:pass' instead of encoding.

    Here and below.

  3. rbtools/api/tests/test_auth_handlers.py (Diff revision 1)
     
     
     
    Show all issues

    Minor nit, but we generally put owner= right after the spy and before the op, so it's better tied to the thing we're spying on. Here and below.

  4. Show all issues

    This could use a Version Added.

  5. Show all issues

    I believe this needs an owner=.

  6. Show all issues

    Same here.

  7. Show all issues

    Same here.

  8. Show all issues

    Same here.

    And the ones below.

  9. Show all issues

    This should be in multi-line form.

  10. Show all issues

    This also needs owner=.

  11. 
      
maubin
david
  1. 
      
  2. rbtools/api/tests/test_auth_handlers.py (Diff revision 2)
     
     
    Show all issues

    Can we call this _TestCommand? Often having classes with the name Test* will generate warnings about collecting test cases.

  3. 
      
maubin
david
  1. 
      
  2. rbtools/api/tests/test_auth_handlers.py (Diff revision 3)
     
     
    Show all issues

    Test docstrings should all start with "Testing"

    This applies throughout this entire change.

  3. rbtools/api/tests/test_auth_handlers.py (Diff revision 3)
     
     
    Show all issues

    This needs a docstring.

  4. rbtools/api/tests/test_auth_handlers.py (Diff revision 3)
     
     
    Show all issues

    Typo: toke -> token

  5. Show all issues

    Instead of "Hit" perhaps we can use "Get"?

  6. Show all issues

    This should use (list[str] | None) = None

  7. 
      
maubin
david
  1. Ship It!
  2. 
      
david
  1. 
      
  2. Show all issues

    Actually, just one note. This can be:

    args=[*command_args, '--debug']

    1. Ah this is leftover debug code, and I meant to only pass --debug for one test. I'll move it instead.

  3. 
      
maubin
Review request changed
Commits:
Summary ID
Add unit tests for our authentication flows.
This adds unit tests for our authentication flows (our HTTP auth handlers andvsession authentication). These flows have recently been updated to include web-based login, so it's a good time to add unit tests for these. There's a TODO note left in here to flesh out the OTP login flow, since this is too outside of the scope of my web-login changes.
e5b4f8e70f3a0baf77f84a6058d2094b8b502e3d
Add unit tests for our authentication flows.
This adds unit tests for our authentication flows (our HTTP auth handlers andvsession authentication). These flows have recently been updated to include web-based login, so it's a good time to add unit tests for these. There's a TODO note left in here to flesh out the OTP login flow, since this is too outside of the scope of my web-login changes.
3cf2f8915595907a3bf5147ec4809cd39bb5574e

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
david
  1. Ship It!
  2.