codeant-ai-for-open-source[bot] commented on PR #36856:
URL: https://github.com/apache/superset/pull/36856#issuecomment-3696789123

   ## Nitpicks 🔍
   
   <table>
   <tr><td>🔒&nbsp;<strong>No security issues identified</strong></td></tr>
   <tr><td>⚡&nbsp;<strong>Recommended areas for review</strong><br><br>
   
   - [ ] <a 
href='https://github.com/apache/superset/pull/36856/files#diff-021730eaa7a0388769fa4bec2a421b292a5aa09002f8f1eb1cbbf2a79b980341R62-R72'><strong>Fragile
 error detection</strong></a><br>The custom metaclass instance check looks for 
the literal substring "Invalid OAuth access token" in the stringified 
exception. This is brittle (case/locale/format differences) and depends on 
wrapping types. Consider making the detection more robust (inspect the 
underlying DatabaseError message or error code) and guard against missing 
attributes.<br>
   
   - [ ] <a 
href='https://github.com/apache/superset/pull/36856/files#diff-961d1d8e8d0e69cf8bfbd39fa3f497c62c2950a150e558c54760f9ded12316a1R469-R495'><strong>Fragile
 request assertion</strong></a><br>The test mocks `requests` and asserts the 
call using `requests.post.assert_called_with(...)`. This is brittle because it 
doesn't assert the number of calls and could silently pass if multiple calls 
happen. Also, it couples to exact call signature ordering. Consider asserting 
call count and inspecting call_args to validate the important pieces (`data` 
payload and `timeout`) rather than relying on a single equality call.<br>
   
   - [ ] <a 
href='https://github.com/apache/superset/pull/36856/files#diff-961d1d8e8d0e69cf8bfbd39fa3f497c62c2950a150e558c54760f9ded12316a1R444-R457'><strong>Typo
 in fixture</strong></a><br>The OAuth2 fixture defines the `scope` value as 
`"refesh_token"` (misspelling). This may cause tests to pass incorrectly or 
mask a bug because the code under test might expect the exact `scope` string 
(e.g., "refresh_token") or validations to succeed/fail differently. Verify 
intended scope and correct the value.<br>
   
   </td></tr>
   </table>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to