Skip to content

Adds a summary for tests #2745

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
Open
wants to merge 6 commits into
base: unstable
Choose a base branch
from
Open

Adds a summary for tests #2745

wants to merge 6 commits into from
+135 −0

Conversation

@sarthakaggarwal97
Copy link
Contributor

@sarthakaggarwal97 sarthakaggarwal97 commented Oct 17, 2025 •
edited
Loading

This is inspired after my discussion with @roshkhatri and @hpatro. This changes allows us to see the results clearly after the suite is run.

The table is printed only when test uses --verbose flag.

Detailed Test Run Report:
+-----------------------+----------------------------------------------------------------+--------+--------+------+
| File                  | Test                                                           | Passed | Failed | Runs |
+-----------------------+----------------------------------------------------------------+--------+--------+------+
| tests/unit/bitops.tcl | BITCOUNT against non-integer value                             |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITCOUNT against test vector #1                                |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITCOUNT against test vector #2                                |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITCOUNT against test vector #3                                |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITCOUNT against test vector #4                                |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITCOUNT against test vector #5                                |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITCOUNT against wrong type                                    |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITCOUNT fuzzing with start/end                                |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITCOUNT fuzzing without start/end                             |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITCOUNT misaligned prefix                                     |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITCOUNT misaligned prefix + full words + remainder            |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITCOUNT regression test for github issue #582                 |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITCOUNT returns 0 against non existing key                    |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITCOUNT returns 0 with negative indexes where start > end     |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITCOUNT returns 0 with out of range indexes                   |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITCOUNT with illegal arguments                                |      0 |      2 |    2 |
| tests/unit/bitops.tcl | BITCOUNT with just start                                       |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITCOUNT with start, end                                       |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITOP and fuzzing                                              |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITOP AND|OR|XOR don't change the string with single input key |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITOP missing key is considered a stream of zero               |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITOP NOT (empty string)                                       |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITOP NOT (known string)                                       |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITOP NOT fuzzing                                              |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITOP or fuzzing                                               |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITOP shorter keys are zero-padded to the key with max length  |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITOP where dest and target are the same key                   |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITOP with empty string after non empty string (issue #529)    |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITOP with integer encoded source objects                      |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITOP with non string source key                               |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITOP xor fuzzing                                              |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITPOS against non-integer value                               |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITPOS against wrong type                                      |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITPOS bit=0 changes behavior if end is given                  |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITPOS bit=0 fuzzy testing using SETBIT                        |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITPOS bit=0 starting at unaligned address                     |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITPOS bit=0 unaligned+full word+reminder                      |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITPOS bit=0 with empty key returns 0                          |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITPOS bit=0 with string less than 1 word works                |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITPOS bit=0 works with intervals                              |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITPOS bit=1 fuzzy testing using SETBIT                        |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITPOS bit=1 returns -1 if string is all 0 bits                |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITPOS bit=1 starting at unaligned address                     |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITPOS bit=1 unaligned+full word+reminder                      |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITPOS bit=1 with empty key returns -1                         |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITPOS bit=1 with string less than 1 word works                |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITPOS bit=1 works with intervals                              |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITPOS will illegal arguments                                  |      2 |      0 |    2 |
| tests/unit/bitops.tcl | BITPOS/BITCOUNT fuzzy testing using SETBIT                     |      2 |      0 |    2 |
| tests/unit/bitops.tcl | Check for memory leaks                                         |      2 |      0 |    2 |
| tests/unit/bitops.tcl | SETBIT/BITFIELD only increase dirty when the value changed     |      2 |      0 |    2 |
+-----------------------+----------------------------------------------------------------+--------+--------+------+

Test Summary: 100 passed, 2 failed

!!! WARNING The following tests failed:

*** [err]: BITCOUNT with illegal arguments in tests/unit/bitops.tcl
Expected 'ERR syntax error' to match 'ERR *syntax1*' (context: type source line 93 file /Users/sarthagg/workspace/valkey/tests/support/test.tcl cmd {assert_match $pattern $error $detail} proc ::assert_error level 1) 
*** [err]: BITCOUNT with illegal arguments in tests/unit/bitops.tcl
Expected 'ERR syntax error' to match 'ERR *syntax1*' (context: type source line 93 file /Users/sarthagg/workspace/valkey/tests/support/test.tcl cmd {assert_match $pattern $error $detail} proc ::assert_error level 1) 
Cleanup: may take some time... OK

@codecov
Copy link

codecov bot commented Oct 17, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.48%. Comparing base (861d079) to head (0517acd).

Additional details and impacted files 
@@             Coverage Diff              @@
##           unstable    #2745      +/-   ##
============================================
+ Coverage     72.43%   72.48%   +0.04%     
============================================
  Files           128      128              
  Lines         70145    70145              
============================================
+ Hits          50813    50843      +30     
+ Misses        19332    19302      -30

see 12 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sarthakaggarwal97 sarthakaggarwal97 marked this pull request as draft October 17, 2025 02:28
@sarthakaggarwal97 sarthakaggarwal97 marked this pull request as ready for review October 17, 2025 02:52
hpatro
hpatro requested changes Oct 17, 2025
View reviewed changes
Copy link
Collaborator

@hpatro hpatro left a comment

Choose a reason for hiding this comment

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

This enhancement will be handy for the team while fixing flaky test failures.

roshkhatri
roshkhatri reviewed Oct 17, 2025
View reviewed changes
@sarthakaggarwal97
Copy link
Contributor Author

thank you @hpatro and @roshkhatri for reviewing. I've addressed the comments!

hpatro
hpatro reviewed Oct 17, 2025
View reviewed changes
roshkhatri
roshkhatri reviewed Oct 17, 2025
View reviewed changes
@hpatro
Copy link
Collaborator

hpatro commented Oct 17, 2025

Was chatting with @roshkhatri further about this. He feels like this might not be that helpful. Rather, test failure count by test name would help me better. Let's hear out others opinion as well.

@roshkhatri
Copy link
Member

Was chatting with @roshkhatri further about this. He feels like this might not be that helpful. Rather, test failure count by test name would help me better. Let's hear out others opinion as well.

Yes, for making it useful for debugging flakiness.

@sarthakaggarwal97
Copy link
Contributor Author

I have updated the PR with the summary output as well, doesn't impact performance, and works with loops too. We can discuss if it is too verbose and we want to hide it behind a parameter.

@roshkhatri
Copy link
Member

This looks great, and its quite verbose and I kinda like it. We can definitely add a parameter to enable this.

@sarthakaggarwal97
test summary
f2a4f1e 
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
@sarthakaggarwal97
addressing some comments
fbdb564 
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
@sarthakaggarwal97
addressing some comments
13f2923 
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
@sarthakaggarwal97
redundant reset
b8ba68a 
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
@sarthakaggarwal97
summary report in the end
c49cd8e 
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
@sarthakaggarwal97
uses verbose flag
0517acd 
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hpatro hpatro hpatro requested changes

@roshkhatri roshkhatri roshkhatri left review comments

Requested changes must be addressed to merge this pull request.

Assignees

@sarthakaggarwal97 sarthakaggarwal97

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sarthakaggarwal97 @hpatro @roshkhatri