Skip to content

Conversation

iamamirsalehi
Copy link
Contributor

@iamamirsalehi iamamirsalehi commented Jun 14, 2025

Summary

This PR refactors the TestBasicCredentials function to use a table-driven testing approach. The goal is to improve test clarity, reduce duplication, and make it easier to extend with new cases in the future.


Changes

  • ✅ Refactored TestBasicCredentials to table-driven format
  • ✅ Combined basic auth and raw credentials checks under unified inputs
  • ✅ Preserved all existing logic including the empty username case
  • ✅ Replaced t.Fatalf with t.Errorf to allow multiple assertion failures per test run

Motivation

Table-driven tests are idiomatic in Go and make it easier to:

  • Maintain and scale tests
  • Add edge cases (e.g., empty password, special characters)
  • Keep tests concise and DRY

Notes

Also included additional edge cases such as:

  • Empty passwords
  • Special characters
  • Long strings
  • Unicode characters

- Empty passwords
- Special characters
- Long strings
- Unicode characters
Copy link
Member

@ndyakov ndyakov left a comment

Choose a reason for hiding this comment

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

@iamamirsalehi thank you for the contribution!

@ndyakov ndyakov merged commit e2295c7 into redis:master Jun 16, 2025
16 checks passed
@iamamirsalehi iamamirsalehi deleted the test/improve-basic-credentials branch June 17, 2025 15:36
ofekshenawa pushed a commit to ofekshenawa/go-redis that referenced this pull request Jun 30, 2025
)

* test: refactor TestBasicCredentials using table-driven tests

* Included additional edge cases:

- Empty passwords
- Special characters
- Long strings
- Unicode characters
ofekshenawa pushed a commit that referenced this pull request Aug 10, 2025
* test: refactor TestBasicCredentials using table-driven tests

* Included additional edge cases:

- Empty passwords
- Special characters
- Long strings
- Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants