Skip to content

[E2E Test] Two-phase review (e2e-two-phase-test-ee827b16)#50

Closed
sourya-deepsource wants to merge 1 commit into
masterfrom
e2e-two-phase-test-ee827b16
Closed

[E2E Test] Two-phase review (e2e-two-phase-test-ee827b16)#50
sourya-deepsource wants to merge 1 commit into
masterfrom
e2e-two-phase-test-ee827b16

Conversation

@sourya-deepsource
Copy link
Copy Markdown
Owner

Automated E2E test. Will be closed automatically.

Comment thread e2e_test_sample.py
import subprocess
import ssl

AWS_SECRET_KEY = "d6s$f9g!j8mg7hw?n&2"
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoded AWS secret key found in source code


Storing secrets like AWS_SECRET_KEY directly in source code is a security risk. Use environment variables or a secrets manager instead.

Comment thread e2e_test_sample.py
self.limits = (1, 10)

def get_number(self, min_max):
raise NotImplemented
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use `NotImplementedError` instead of `NotImplemented`


NotImplemented is a special singleton used for binary operator fallbacks, not for signalling unimplemented methods. Raise NotImplementedError instead.

@sourya-deepsource sourya-deepsource deleted the e2e-two-phase-test-ee827b16 branch March 10, 2026 11:00
@deepsource-development
Copy link
Copy Markdown

deepsource-development Bot commented Mar 10, 2026

DeepSource Code Review

We reviewed changes in 7f7058a...c79cd1c on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade  

Focus Area: Reliability
Security  

Reliability  

Complexity  

Hygiene  

Feedback

  • Unfinalized test scaffolding
    • The e2e test file keeps template leftovers — a hardcoded secret, unused ssl imports, stubbed raises, and methods that never use self. Finish or remove scaffolding: eliminate hardcoded secrets/imports, implement or drop stubs, and convert utilities to static/helpers.
  • Confused use of NotImplemented vs exceptions
    • A sentinel value was treated like an exception, leading to invalid raises and runtime failures. Reserve NotImplemented for operator fallthrough, raise NotImplementedError for unimplemented abstract behavior, and choose explicit exception types for misuse.
  • Mixed responsibilities causing dead code
    • Credentials, setup utilities and test logic are intermixed, so unused imports and irrelevant instance methods persist. Separate concerns: move config/credentials to fixtures or environment, extract helpers into a utility module, and remove unused symbols.

Code Review Summary

Analyzer Status Updated (UTC) Details
Python Mar 10, 2026 11:00a.m. Review ↗
Secrets Mar 10, 2026 11:00a.m. Review ↗

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