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: fix ioredis tests #830

Merged
merged 6 commits into from
Jan 14, 2022
Merged

Conversation

rauno56
Copy link
Member

@rauno56 rauno56 commented Jan 13, 2022

Which problem is this PR solving?

Some of the assertions are done in the hooks in the tests for ioredis.

  1. The errors thrown in the hooks are eaten, so no benefit having them there,
  2. Even if they aren't one of them was validating an ever-increasing counter, which can ever only be correct once,
  3. Test for scan broke when there were more than 10 keys in the DB - not a problem when running on a clean DB every time, but happens locally when tests occasionally completely fail and the cleanup will not happen.

Short description of the changes

  • Added sinon to move the assertions in the hooks outside of them.
  • Tweaked scan tests with match and count parameters
  • Narrowed down the TAV version set - if we expect devs to ever run them, it better not take an hour. It currently takes 2 minutes.

@rauno56 rauno56 requested a review from a team January 13, 2022 13:01
@rauno56 rauno56 mentioned this pull request Jan 13, 2022
6 tasks
@codecov
Copy link

codecov bot commented Jan 13, 2022

Codecov Report

Merging #830 (13f70e9) into main (88db0a4) will increase coverage by 0.24%.
The diff coverage is n/a.

❗ Current head 13f70e9 differs from pull request most recent head 71bf972. Consider uploading reports for the commit 71bf972 to get more accurate results

@@            Coverage Diff             @@
##             main     #830      +/-   ##
==========================================
+ Coverage   94.90%   95.15%   +0.24%     
==========================================
  Files          13       18       +5     
  Lines         707     1299     +592     
  Branches      142      176      +34     
==========================================
+ Hits          671     1236     +565     
- Misses         36       63      +27     
Impacted Files Coverage Δ
...opentelemetry-instrumentation-ioredis/src/utils.ts 100.00% <0.00%> (ø)
...metry-instrumentation-ioredis/test/ioredis.test.ts 95.87% <0.00%> (ø)
...ages/auto-instrumentations-node/test/utils.test.ts 96.87% <0.00%> (ø)
...try-instrumentation-ioredis/src/instrumentation.ts 91.46% <0.00%> (ø)
...tapackages/auto-instrumentations-node/src/utils.ts 96.77% <0.00%> (ø)

@rauno56 rauno56 merged commit fcdc14e into open-telemetry:main Jan 14, 2022
@rauno56 rauno56 deleted the fix/ioredis-tests branch January 14, 2022 12:32
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