seasocks icon indicating copy to clipboard operation
seasocks copied to clipboard

Support for fragmented Messages

Open A-J-Bauer opened this issue 7 years ago • 9 comments

Added support for fragmented messages as described in RFC 6455 - Page 27 - 5.4.

A-J-Bauer avatar Dec 05 '18 10:12 A-J-Bauer

Codecov Report

Merging #103 into master will decrease coverage by 0.87%. The diff coverage is 19.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #103      +/-   ##
==========================================
- Coverage   39.23%   38.36%   -0.88%     
==========================================
  Files          41       41              
  Lines        2039     2075      +36     
==========================================
- Hits          800      796       -4     
- Misses       1239     1279      +40
Impacted Files Coverage Δ
src/main/c/seasocks/Connection.h 23.07% <ø> (ø) :arrow_up:
src/main/c/Connection.cpp 21.88% <0%> (-0.41%) :arrow_down:
src/main/c/internal/HybiPacketDecoder.h 100% <100%> (ø) :arrow_up:
src/main/c/HybiPacketDecoder.cpp 55.69% <26.92%> (-28.79%) :arrow_down:

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 eb54d16...a801e02. Read the comment docs.

codecov[bot] avatar Dec 05 '18 10:12 codecov[bot]

Codecov Report

Merging #103 into master will decrease coverage by 0.45%. The diff coverage is 19.56%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #103      +/-   ##
==========================================
- Coverage   39.23%   38.78%   -0.46%     
==========================================
  Files          41       41              
  Lines        2039     2073      +34     
==========================================
+ Hits          800      804       +4     
- Misses       1239     1269      +30
Impacted Files Coverage Δ
src/main/c/seasocks/Connection.h 23.07% <ø> (ø) :arrow_up:
src/main/c/Connection.cpp 21.88% <0%> (-0.41%) :arrow_down:
src/main/c/internal/HybiPacketDecoder.h 100% <100%> (ø) :arrow_up:
src/main/c/HybiPacketDecoder.cpp 67.53% <28%> (-16.96%) :arrow_down:

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 eb54d16...7b7e0f2. Read the comment docs.

codecov[bot] avatar Dec 05 '18 10:12 codecov[bot]

The CI build doesn't tell much why the tests fail. We should improve this (though that's not part of this PR).

Update: Issue opened #104

offa avatar Dec 05 '18 12:12 offa

Guess the check now fails because Valgrind complains. It would be helpful if the memory logs would be accessible through a link. I will spend some more time using Valgrind locally and if I don't find anything that can be fixed in a reasonable amount of time I have to leave it alone.

A-J-Bauer avatar Dec 05 '18 12:12 A-J-Bauer

Guess the check now fails because Valgrind complains. It would be helpful if the memory logs would be accessible through a link.

Indeed. For now, you can run the ctest commands from the .travis.yml locally.

offa avatar Dec 05 '18 13:12 offa

Hmm … running the unit tests locally (via AllTests) showed up some failures:

test cases:  61 |  53 passed |  8 failed
assertions: 198 | 145 passed | 53 failed

ctest -D ExperimentalBuild -j${JOBS} (CI and locally) doesn't show those; ctest without further arguments fails too.

offa avatar Dec 05 '18 13:12 offa

@A-J-Bauer I've improved the test execution on Travis. The tests are now executed correctly and show what's wrong on failure. Please rebase onto master to get your PR updated.

offa avatar Dec 05 '18 13:12 offa

@offa Thank you for the improved test execution, pointed me in the right direction. I removed a check for unmasked client messages which broke tests. Now travis seems to be happy. Codecov still isn't.

A-J-Bauer avatar Dec 05 '18 20:12 A-J-Bauer

@offa Thank you for the improved test execution, pointed me in the right direction. I removed a check for unmasked client messages which broke tests. Now travis seems to be happy.

:+1: You can rebase onto the updated master, so you get an improved output when updating your PR here.

Codecov still isn't.

The newly added code isn't covered by tests. Please visit the codecov link, it already provides a useful overview of uncovered lines – no need to struggle with building coverage data locally.

The Hybi related tests are all in HybiTests.cpp; connection related in ConnectionTests.cpp.

Please let me know, if you need any help here.

offa avatar Dec 06 '18 13:12 offa