Skip to content

Conversation

trentm
Copy link
Contributor

@trentm trentm commented Jun 26, 2025

In [email protected] and earlier versions of redis v4, client.disconnect()
will throw if the connect failed. That broke the "with empty string for
client URL, ..." test case, at least on macOS:

npm run test:docker:run
RUN_REDIS_TESTS=1 npm t

I cannot explain why this is not failing in CI. Perhaps something
platform specific? This is related to socket handling in the redis
client. I believe the relevant change in node-redis was:
redis/node-redis#2295
which was part of @redis/[email protected] which was included in
[email protected].

In [email protected] and earlier versions of redis v4, client.disconnect()
will throw if the connect failed. That broke the "with empty string for
client URL, ..." test case, at least on macOS:

    npm run test:docker:run
    RUN_REDIS_TESTS=1 npm t

I cannot explain why this is not failing in CI. Perhaps something
platform specific? This is related to socket handling in the redis
client. I believe the relevant change in node-redis was:
redis/node-redis#2295
which was part of `@redis/[email protected]` which was included in
`[email protected]`.
@trentm trentm self-assigned this Jun 26, 2025
@trentm trentm requested a review from a team as a code owner June 26, 2025 22:07
@github-actions github-actions bot requested a review from blumamir June 26, 2025 22:07
@trentm trentm changed the title test(redis-4): fix tests with [email protected] and earlier test(instrumentation-redis-4): fix tests with [email protected] and earlier Jun 26, 2025
Copy link

codecov bot commented Jun 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.81%. Comparing base (c8fa46d) to head (69e05d6).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2914      +/-   ##
==========================================
+ Coverage   89.76%   89.81%   +0.04%     
==========================================
  Files         187      187              
  Lines        9149     9149              
  Branches     1885     1885              
==========================================
+ Hits         8213     8217       +4     
+ Misses        936      932       -4     

see 1 file 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.

@trentm trentm enabled auto-merge (squash) June 27, 2025 21:49
@trentm trentm merged commit b60b06e into open-telemetry:main Jun 27, 2025
23 checks passed
@trentm trentm deleted the trentm-fix-instr-redis-4-test branch June 27, 2025 22:07
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.

4 participants