Skip to content

Conversation

@yoav-steinberg
Copy link
Contributor

@yoav-steinberg yoav-steinberg commented Sep 25, 2021

Fixing CI test issues introduced in #8687

  • valgrind warnings in readQueryFromClient when client was freed by processInputBuffer
  • adding DEBUG pause-cron [1|0] for tests not to be time dependent.
  • skipping a test that depends on socket buffers / events not compatible with TLS
  • making sure client got subscribed before publishing to it by not using deferring client

To resolve timing issue of test taking more than 1sec and therefore failing on slow CI platforms.
Because signal hack doesn't force all sent commands to be handled before sleep when using TLS.

Added tls skip mechanism to test framework.
If client was freed in processCommand we need to propagate the error out put processInputBuffer so we know not to access the client struct anymore (in beforeNextClient).
@oranagra
Copy link
Member

Added `debug pause-cron [0|1]` command and use it to disable cron based client eviction during test.
@oranagra
Copy link
Member

partial daily CI triggered: https://github.com/redis/redis/actions/runs/1275199158

@oranagra oranagra merged commit 6600253 into redis:unstable Sep 26, 2021
@oranagra
Copy link
Member

@yoav-steinberg it seems that there are still some sporadic issues: https://github.com/redis/redis/runs/3726678420?check_suite_focus=true

madolson added a commit to madolson/redis that referenced this pull request Feb 21, 2023
oranagra pushed a commit that referenced this pull request Feb 28, 2023
warrick1016 pushed a commit to ctripcorp/Redis-On-Rocks that referenced this pull request Sep 2, 2025
Fixing CI test issues introduced in redis#8687
- valgrind warnings in readQueryFromClient when client was freed by processInputBuffer
- adding DEBUG pause-cron for tests not to be time dependent.
- skipping a test that depends on socket buffers / events not compatible with TLS
- making sure client got subscribed by not using deferring client
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants