search

reaktivity / k3po-nukleus-ext.java

K3PO Nukleus Extension
Apache License 2.0
0 stars 9 forks source link

K3po nukleus extension tests #20

Closed NicoletaOita closed 7 years ago

NicoletaOita commented 7 years ago

Added k3po nukleus tests for window and throttle options for each of the transmissions modes (simplex, half-duplex, duplex).

dpwspoon commented 7 years ago

These look good to me minus the confusion on the one test. Please change that to pass (I think). @jfallows do you confirm?

NicoletaOita commented 7 years ago

@dpwspoon I've updated the tests as requested and added a note in the readme related to the window and throttle options. Please merge if it's ok.

cmebarrow commented 7 years ago

@NicoletaOita I don't understand how any tests in k3po nukleus extension can be related to TCP. Please change the title of this PR to state what it is really doing and add a description if needed.

NicoletaOita commented 7 years ago

@cmebarrow Indeed, these are not tcp tests, I've started to look over them as part of the tcp story, but they are k3po-nukleus-extension tests. I've updated the name and the description of the story.

NicoletaOita commented 7 years ago

@cmebarrow Thank you for the feedback, I've applied it: updated the readme, replaced nukleus:throttle "off" with nukleus:throttle "none", removed the tests. However, there is one I think you've missed, SimplexIT.shouldThrottleInitialOnlyClientSentDataTest. I've added it just to ask about the expected behavior and what exactly does the option "update" do. So if I have a long message such that the initial window is filled up and "update" is set to none, should it result in not receiving the rest of the message?

cmebarrow commented 7 years ago

@NicoletaOita Regarded your last question, yes I would expect it would result in not all the data being sent. So I would expect that test to fail with timeout. Is that not what you observe?

NicoletaOita commented 7 years ago

@cmebarrow The test was passing as it was initially and I was expecting it to fail with timeout. After changing the scripts as you said, the test is failing because no reset is sent (no"write aborted") and I would expect it to pass. Please merge this PR if it's ok (I've applied all the feedback except for the window option which causes error if removed) and we'll track this issue here.

cmebarrow commented 7 years ago

@jfallows or @dpwspoon Please do final review and merge

NicoletaOita commented 7 years ago

@dpwspoon The requested changes left which appear above don't apply anymore . I've removed that test according to Chris's feedback.