Skip to content

Conversation

@hezean
Copy link
Contributor

@hezean hezean commented Dec 4, 2022

resolves #27

@hezean hezean marked this pull request as draft December 4, 2022 17:51
@codecov-commenter
Copy link

codecov-commenter commented Dec 4, 2022

Codecov Report

Merging #29 (284fde5) into main (54af79b) will decrease coverage by 0.97%.
The diff coverage is 50.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main      #29      +/-   ##
============================================
- Coverage     76.74%   75.77%   -0.98%     
- Complexity      268      271       +3     
============================================
  Files            31       31              
  Lines           972     1003      +31     
  Branches         68       70       +2     
============================================
+ Hits            746      760      +14     
- Misses          184      200      +16     
- Partials         42       43       +1     
Impacted Files Coverage Δ
...java/com/featureprobe/sdk/server/FeatureProbe.java 64.46% <ø> (ø)
...ain/java/com/featureprobe/sdk/server/FPConfig.java 56.60% <20.00%> (-3.82%) ⬇️
...in/java/com/featureprobe/sdk/server/FPContext.java 66.00% <50.00%> (-1.40%) ⬇️
...m/featureprobe/sdk/server/PollingSynchronizer.java 59.21% <55.17%> (-3.76%) ⬇️

@hezean hezean marked this pull request as ready for review December 27, 2022 06:09
@SSebo
Copy link
Member

SSebo commented Dec 28, 2022

Good Job, How about add a Test case to connecting featureprobe.io to make sure socketio connect success?

@hezean
Copy link
Contributor Author

hezean commented Dec 28, 2022 via email

@SSebo
Copy link
Member

SSebo commented Dec 28, 2022

it was manually tested, imho it’s not a good idea to let ut depends on external resourcesBest regards,Zean HeSent from mobile device.On Dec 28, 2022, at 10:49, SSebo @.> wrote: Good Job, How about add a Test case to connecting featureprobe.io to make sure socketio connect success? —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.>

You are right, but it is better than no test. Rust SDK test this via an embed Server.
I have two ideas:

  1. Treat it as an integration test, only run in github actions.
  2. Embed js socketio server for test (better but more complex).

@gangb-tech
Copy link
Member

Good, The stream feature should be independent. You can try to abstract a StreamSynchronizer to implement it.

@gangb-tech
Copy link
Member

This request has been merged into #36

@gangb-tech gangb-tech closed this Mar 6, 2023
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.

feat: java sdk integrate socketio client to receive realtime toggle update event

4 participants