Skip to content

[USER-027][VALI-004] Add login test cases and refine P01_hello.py #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Sindhu1702013 merged 4 commits into from
Feb 3, 2026
Merged

[USER-027][VALI-004] Add login test cases and refine P01_hello.py #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b4ebea3
Added changes
Sindhu1702013 Feb 3, 2026
be76e76
Added changes
Sindhu1702013 Feb 3, 2026
1fdbe57
PR changes
Sindhu1702013 Feb 3, 2026
fcf8ffd
Commit changes
Sindhu1702013 Feb 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion Programs/P01_hello.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,15 @@ def justPrint(text):
'''This function prints the text passed as argument to this function'''
print(text)
base_value = 10
b=20
increment_value=20
difference = increment_value - base_value
divide_value = increment_value / base_value
Copy link
Contributor

@appmod-pr-genie appmod-pr-genie bot Feb 3, 2026

Choose a reason for hiding this comment

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

Warning Confidence Score: 90%

Security/Robustness: Potential ZeroDivisionError

The new division operation uses base_value as a divisor. While currently hardcoded to 10, if this logic is later modified to accept dynamic input, it could cause a crash if the divisor is zero.

Suggested change
divide_value = increment_value / base_value
divide_value = increment_value / base_value if base_value != 0 else 0
Reasons & Gaps

Reasons

  1. Division by zero is a common runtime error that leads to application crashes
  2. Implementing a check ensures the function remains robust if inputs become dynamic
  3. Prevents unhandled exceptions in mathematical operations

Gaps

  1. The values are currently hardcoded constants, making the risk theoretical in the current static context.

print("Difference is:", difference)
print("Divide value is:", divide_value)

if __name__ == '__main__':
justPrint('Hello Sindhuja')
justPrint('Hello Sindhuja')
justPrint('Hello Sindhuja')


3 changes: 3 additions & 0 deletions Programs/login.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,6 @@ def login(self, email, password):
print(app.login("wrong@test.com", "123")) # Test case 2
print(app.login("wrong@test.com", "123")) # Test case 3 (locks account)
print(app.login("user@test.com", "Password@123")) # Should fail because account is locked
print("TC4:", app.login("wrong@test.com", "123")) # This will now lock
Copy link
Contributor

@appmod-pr-genie appmod-pr-genie bot Feb 3, 2026

Choose a reason for hiding this comment

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

Warning Confidence Score: 95%

I notice a bit of a contradiction in the test logic here. Line 31's comment says it locks the account, and line 32 expects a locked state, but then line 33's comment says 'This will now lock'. This inconsistency makes it unclear what the actual locking threshold is and whether the tests are validating the correct state. We should align the comments and sequence to reflect the intended behavior.

Suggested change
print("TC4:", app.login("wrong@test.com", "123")) # This will now lock
print("TC4:", app.login("wrong@test.com", "123")) # Attempt on already locked account

print("TC5:", app.login("user@test.com", "Password@123")) # Locked account case
print("TC6:", app.login("user@test.com", "Password@123")) # Locked account case
Comment on lines +34 to +35
Copy link
Contributor

@appmod-pr-genie appmod-pr-genie bot Feb 3, 2026

Choose a reason for hiding this comment

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

Warning Confidence Score: 100%

Hardcoded Test Credentials

I see the issue of hardcoded credentials like user@test.com and Password@123 is still present from the last review, and more instances have been added. Committing any credentials to version control is a security risk as it can expose username formats and password patterns.

Let's move these test credentials out of the code. They should be loaded from a configuration file (e.g., .env, config.ini) that is excluded from version control via .gitignore, or injected as environment variables during test execution.

Copy link
Contributor

@appmod-pr-genie appmod-pr-genie bot Feb 3, 2026

Choose a reason for hiding this comment

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

JAS Confidence Score: 100%

JAS - Just a suggestion
I spotted that TC5 and TC6 are performing the exact same check—logging in with correct credentials on a locked account. Since they use the same parameters and expect the same state, TC6 doesn't add any new coverage. We can remove the duplicate to keep the test suite clean and efficient.

Loading