Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1017973002:
Implement working tests for the simple QUIC client classes. (Closed)
|
Created:
5 years, 9 months ago by Ryan Hamilton Modified:
5 years, 9 months ago CC:
chromium-reviews, cbentzel+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionImplement working tests for the simple QUIC client classes.
Committed: https://crrev.com/a6d42cd433e067ca3a35b0a09ea8512be769477c
Cr-Commit-Position: refs/heads/master@{#321704}
Patch Set 1 #
Total comments: 2
Patch Set 2 : fixed #Patch Set 3 : Rebase #Patch Set 4 : net export #Patch Set 5 : no epoll #Patch Set 6 : gn!? #Patch Set 7 : better GN fix #Patch Set 8 : more exports #Patch Set 9 : Rebase #Patch Set 10 : Add simple_quic_tools as a dep for net_unittests #Patch Set 11 : no export #
Total comments: 4
Patch Set 12 : cleanup #
Messages
Total messages: 54 (31 generated)
rch@chromium.org changed reviewers: + rtenneti@chromium.org
lgtm https://codereview.chromium.org/1017973002/diff/1/net/net.gypi File net/net.gypi (right): https://codereview.chromium.org/1017973002/diff/1/net/net.gypi#newcode1699 net/net.gypi:1699: 'tools/quic/quic_simple_client_stream_test.cc', nit: sort order tools/quic/quic_simple_client_session_test.cc tools/quic/quic_simple_client_stream_test.cc tools/quic/quic_simple_client_test.cc
The CQ bit was checked by rch@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rtenneti@chromium.org Link to the patchset: https://codereview.chromium.org/1017973002/#ps40001 (title: "Rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1017973002/40001
https://codereview.chromium.org/1017973002/diff/1/net/net.gypi File net/net.gypi (right): https://codereview.chromium.org/1017973002/diff/1/net/net.gypi#newcode1699 net/net.gypi:1699: 'tools/quic/quic_simple_client_stream_test.cc', On 2015/03/19 20:52:19, ramant wrote: > nit: sort order > tools/quic/quic_simple_client_session_test.cc > tools/quic/quic_simple_client_stream_test.cc > tools/quic/quic_simple_client_test.cc Done.
The CQ bit was unchecked by rch@chromium.org
The CQ bit was checked by rch@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rtenneti@chromium.org Link to the patchset: https://codereview.chromium.org/1017973002/#ps60001 (title: "net export")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1017973002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by rch@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rtenneti@chromium.org Link to the patchset: https://codereview.chromium.org/1017973002/#ps80001 (title: "no epoll")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1017973002/80001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by rch@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rtenneti@chromium.org Link to the patchset: https://codereview.chromium.org/1017973002/#ps100001 (title: "gn!?")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1017973002/100001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_gn_dbg on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by rch@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rtenneti@chromium.org Link to the patchset: https://codereview.chromium.org/1017973002/#ps120001 (title: "better GN fix")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1017973002/120001
The CQ bit was checked by rch@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rtenneti@chromium.org Link to the patchset: https://codereview.chromium.org/1017973002/#ps140001 (title: "more exports")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1017973002/140001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
The CQ bit was checked by rch@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rtenneti@chromium.org Link to the patchset: https://codereview.chromium.org/1017973002/#ps160001 (title: "Rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1017973002/160001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...) ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
The CQ bit was checked by rch@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rtenneti@chromium.org Link to the patchset: https://codereview.chromium.org/1017973002/#ps180001 (title: "Add simple_quic_tools as a dep for net_unittests")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1017973002/180001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
The CQ bit was checked by rch@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rtenneti@chromium.org Link to the patchset: https://codereview.chromium.org/1017973002/#ps200001 (title: "no export")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1017973002/200001
thakis@chromium.org changed reviewers: + thakis@chromium.org
(If simple_quic_tools ever gets linked into net, it needs to become a component – else it'll end up twice in net_unittests: Once via net.dll, once because net_unittests explicitly depends on simple_quic_tools.) https://codereview.chromium.org/1017973002/diff/200001/net/tools/quic/quic_si... File net/tools/quic/quic_simple_client.h (right): https://codereview.chromium.org/1017973002/diff/200001/net/tools/quic/quic_si... net/tools/quic/quic_simple_client.h:44: public QuicPacketReader::Visitor { ? https://codereview.chromium.org/1017973002/diff/200001/net/tools/quic/quic_si... File net/tools/quic/quic_simple_client_session.h (right): https://codereview.chromium.org/1017973002/diff/200001/net/tools/quic/quic_si... net/tools/quic/quic_simple_client_session.h:28: : public QuicClientSessionBase { ?
The CQ bit was unchecked by rch@chromium.org
> (If simple_quic_tools ever gets linked into net, it needs > to become a component – else it'll end up twice in > net_unittests: Once via net.dll, once because > net_unittests explicitly depends on simple_quic_tools.) *nod* If this goes into net, I'll do that my actually moving the files into the net target (and into net/quic/ instead of net/tools/quic/). But to make sure I understand what you mean, would setting "'type': '<(component)'," make it a component? https://codereview.chromium.org/1017973002/diff/200001/net/tools/quic/quic_si... File net/tools/quic/quic_simple_client.h (right): https://codereview.chromium.org/1017973002/diff/200001/net/tools/quic/quic_si... net/tools/quic/quic_simple_client.h:44: public QuicPacketReader::Visitor { On 2015/03/21 03:59:53, Nico wrote: > ? Gah! I removed NET_EXPORT_PRIVATE but forgot to fix the indentation. Thanks! https://codereview.chromium.org/1017973002/diff/200001/net/tools/quic/quic_si... File net/tools/quic/quic_simple_client_session.h (right): https://codereview.chromium.org/1017973002/diff/200001/net/tools/quic/quic_si... net/tools/quic/quic_simple_client_session.h:28: : public QuicClientSessionBase { On 2015/03/21 03:59:53, Nico wrote: > ? *facepalm* Same thing. Fixed.
Patchset #12 (id:220001) has been deleted
The CQ bit was checked by rch@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rtenneti@chromium.org Link to the patchset: https://codereview.chromium.org/1017973002/#ps240001 (title: "cleanup")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1017973002/240001
Message was sent while issue was closed.
Committed patchset #12 (id:240001)
Message was sent while issue was closed.
Patchset 12 (id:??) landed as https://crrev.com/a6d42cd433e067ca3a35b0a09ea8512be769477c Cr-Commit-Position: refs/heads/master@{#321704} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
