Handle NaN in toDouble.

[krm1312]

#1482

[codecov]

Codecov Report

Merging #1483 into 6.0.x will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##              6.0.x    #1483      +/-   ##
============================================
+ Coverage     78.82%   78.86%   +0.04%     
- Complexity     6191     6196       +5     
============================================
  Files           460      460              
  Lines         20702    20704       +2     
  Branches       2280     2281       +1     
============================================
+ Hits          16318    16329      +11     
+ Misses         3331     3323       -8     
+ Partials       1053     1052       -1     

Impacted Files	Coverage Δ	Complexity Δ
.../java/io/lettuce/core/internal/LettuceStrings.java	64.51% <100.00%> (+5.89%)	19.00 <0.00> (+4.00)
.../java/io/lettuce/core/protocol/CommandHandler.java	73.58% <0.00%> (-1.80%)	109.00% <0.00%> (-4.00%)
...a/io/lettuce/core/protocol/ConnectionWatchdog.java	78.43% <0.00%> (-1.31%)	39.00% <0.00%> (ø%)
...java/io/lettuce/core/protocol/DefaultEndpoint.java	70.06% <0.00%> (+0.46%)	99.00% <0.00%> (ø%)
.../io/lettuce/core/dynamic/ReactiveTypeAdapters.java	88.26% <0.00%> (+0.86%)	1.00% <0.00%> (ø%)
...main/java/io/lettuce/core/AbstractRedisClient.java	82.88% <0.00%> (+1.06%)	49.00% <0.00%> (+1.00%)
...ce/core/masterreplica/SentinelTopologyRefresh.java	86.27% <0.00%> (+1.30%)	35.00% <0.00%> (ø%)
...e/core/masterreplica/SentinelTopologyProvider.java	83.78% <0.00%> (+2.70%)	10.00% <0.00%> (+1.00%)
...in/java/io/lettuce/core/masterreplica/Timeout.java	75.00% <0.00%> (+12.50%)	4.00% <0.00%> (+1.00%)
... and 1 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c004e85...562d64d. Read the comment docs.

[dengliming]

Thanks. LGTM

[dengliming]

Seems we should merge to main branch? @mp911de

[mp911de]

There's no test attached. Can you @dengliming create one during the merge? We should port the change (via git cherry-pick) to 5.3.x and main.

FTR, edit the commit message to match the Lettuce commit message format (here are two commits for a PR merge: 5810f96 and b240c58).

[dengliming]

Can we just add a simple test for method toDouble? I haven't find a command returning like nan this. This case only happen in redis-search (redis module). WDYT? @mp911de

[mp911de]

That's perfectly fine to go with a unit test as that's the simple-most approach.

[krm1312]

Thanks @dengliming @mp911de . Let me know if you need me to clean anything up or otherwise change anything. I put PR to 6.0.x because that's the release branch I was hoping it would get in. Further back is of course fine w/ me as well.

[dengliming]

@krm1312 It would be better If you can also add a unit test for LettuceStrings.toDouble

[dengliming]

Thanks. Looks good. a small suggestion. in order to show your id in the contributor list, don’t forget the configurations below:

git config --global user.name "username"
git config --global user.email "[email protected]"

[mp911de]

One more thing: please squash your commits into a single one. Merge commits cause trouble during cherry picking.

[dengliming]

@krm1312 Thanks for your contribution. That's merged.

[dengliming]

@mp911de LettuceStrings This file is in conflict with the version 5.3.x and the package name is also different. Do I need to keep it?

[mp911de]

Great merge, @dengliming 👍

Fo 5.3.x, we need to apply the fix manually, since we can't backport the commit.

[dengliming]

@mp911de Got it. That‘s backported now.