Skip to content

test: splitted redis tests to default, cluser and sentinel #2123

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

Jump to bottom
Merged
abhilash-sivan merged 6 commits into from
Nov 10, 2025
Merged

test: splitted redis tests to default, cluser and sentinel #2123

abhilash-sivan merged 6 commits into from
Nov 10, 2025

Conversation

@abhilash-sivan
Copy link
Contributor

@abhilash-sivan abhilash-sivan commented Nov 6, 2025 •
edited
Loading

Splitted redist rests into 3 subtests:

  • redis/default_test
  • redis/cluster_test
  • redis/sentinel_test

Checks

10 groups for database

9 groups for database

@abhilash-sivan abhilash-sivan force-pushed the test-split-resis branch 3 times, most recently from b5eb3d0 to 825f479 Compare November 7, 2025 07:10
@abhilash-sivan abhilash-sivan marked this pull request as ready for review November 7, 2025 07:10
@abhilash-sivan abhilash-sivan requested a review from a team as a code owner November 7, 2025 07:10
aryamohanan
aryamohanan reviewed Nov 7, 2025
View reviewed changes
Copy link
Contributor

@aryamohanan aryamohanan left a comment

Choose a reason for hiding this comment

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

Are we duplicating all the tests across these three files? If so IMO, that’s not a good approach, it’ll make adding or maintaining tests in the future much more difficult.

@abhilash-sivan
Copy link
Contributor Author

abhilash-sivan commented Nov 7, 2025

Are we duplicating all the tests across these three files? If so IMO, that’s not a good approach, it’ll make adding or maintaining tests in the future much more difficult.

Yes, I agree... removed duplicates now

aryamohanan
aryamohanan approved these changes Nov 7, 2025
View reviewed changes
Copy link
Contributor

@aryamohanan aryamohanan left a comment

Choose a reason for hiding this comment

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

Wow

abhilash-sivan
abhilash-sivan commented Nov 7, 2025
View reviewed changes
abhilash-sivan
abhilash-sivan commented Nov 7, 2025
View reviewed changes
@abhilash-sivan abhilash-sivan force-pushed the test-split-resis branch 3 times, most recently from c9bad7b to 30901c0 Compare November 7, 2025 14:49
kirrg001
kirrg001 requested changes Nov 7, 2025
View reviewed changes
Copy link
Contributor

@kirrg001 kirrg001 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

All database test groups (1-10) run the same tests now?

packages/collector/test/tracing/databases/redis/cluster_test.js
packages/collector/test/tracing/databases/redis/default_test.js
packages/collector/test/tracing/databases/redis/sentinel_test.js

See latest build.

@abhilash-sivan
Copy link
Contributor Author

abhilash-sivan commented Nov 7, 2025

All database test groups (1-10) run the same tests now?

packages/collector/test/tracing/databases/redis/cluster_test.js
packages/collector/test/tracing/databases/redis/default_test.js
packages/collector/test/tracing/databases/redis/sentinel_test.js

See latest build.

No, it's not!!

split-7 cluster_test.js exclusively
split-8 default_test.js exclusively
split-9 sentinel_test.js exclusively

other splits: other database tests
ps: split-2 has ioredis, but not exclusive; it has elasticsearch also.

Please confirm

@kirrg001
Copy link
Contributor

kirrg001 commented Nov 10, 2025

Yeah you are right, these are the modified files. My bad 🛁

kirrg001
kirrg001 reviewed Nov 10, 2025
View reviewed changes
@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 10, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

kirrg001
kirrg001 approved these changes Nov 10, 2025
View reviewed changes
Copy link
Contributor

@kirrg001 kirrg001 left a comment

Choose a reason for hiding this comment

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

Great 👍

@abhilash-sivan abhilash-sivan merged commit 982ad93 into main Nov 10, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kirrg001 kirrg001 kirrg001 approved these changes

@aryamohanan aryamohanan aryamohanan approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@abhilash-sivan @kirrg001 @aryamohanan