Skip to content
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.

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

Test coverage for ECHO for reply schema validation #1549

Merged
merged 2 commits into from
Jan 13, 2025

Conversation

zuiderkwast
Copy link
Contributor

After #1545 disabled some tests for reply schema validation, we now have another issue that ECHO is not covered.

WARNING! The following commands were not hit at all:
  echo
ERROR! at least one command was not hit by the tests

(https://github.com/valkey-io/valkey/actions/runs/12728730819/job/35479652660)

This patch adds a test case for ECHO in the unit/other test suite. I haven't checked if there are more commands that aren't covered.

@zuiderkwast zuiderkwast added test-failure An issue indicating a test failure run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP) labels Jan 12, 2025
Copy link

codecov bot commented Jan 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.93%. Comparing base (ad592f7) to head (2d130e0).
Report is 1 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #1549      +/-   ##
============================================
- Coverage     71.00%   70.93%   -0.07%     
============================================
  Files           120      120              
  Lines         65061    65061              
============================================
- Hits          46194    46154      -40     
- Misses        18867    18907      +40     

see 14 files with indirect coverage changes

@enjoy-binbin enjoy-binbin merged commit dc9ca1b into valkey-io:unstable Jan 13, 2025
56 of 58 checks passed
@@ -1220,7 +1220,7 @@ jobs:
if: |
(github.event_name == 'workflow_dispatch' ||
(github.event_name == 'schedule' && github.repository == 'valkey-io/valkey') ||
(github.event_name == 'pull_request' && github.event.pull_request.base.ref != 'unstable')) &&
(github.event_name == 'pull_request' && (contains(github.event.pull_request.labels.*.name, 'run-extra-tests') || github.event.pull_request.base.ref != 'unstable'))) &&
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@enjoy-binbin I was going to revert this change before merging. Sorry i didn't mention it.

I guess it's fine to run schema validator in run-extra-tests, but it takes one hour to run.

@zuiderkwast zuiderkwast deleted the test-coverage-echo branch January 13, 2025 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP) test-failure An issue indicating a test failure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants