Skip to content

quiche: Move a bunch of quic testonly platform libraries to //test.#6693

Merged
htuch merged 1 commit intoenvoyproxy:masterfrom
wu-bin:quic_platform_test_lib
Apr 25, 2019
Merged

quiche: Move a bunch of quic testonly platform libraries to //test.#6693
htuch merged 1 commit intoenvoyproxy:masterfrom
wu-bin:quic_platform_test_lib

Conversation

@wu-bin
Copy link
Copy Markdown
Contributor

@wu-bin wu-bin commented Apr 24, 2019

Description:

Move the following QUIC testonly platform libraries to //test:

  • quic_expect_bug_impl
  • quic_mock_log_impl
  • quic_test_impl
  • quic_test_output_impl

Risk Level: none, only moved unused code.
Testing:

bazel test --test_output=all test/extensions/quic_listeners/quiche/platform:all @com_googlesource_quiche//:all

Docs Changes: none
Release Notes: none

Signed-off-by: Bin Wu <wub@google.com>
@wu-bin wu-bin marked this pull request as ready for review April 24, 2019 12:11
@wu-bin
Copy link
Copy Markdown
Contributor Author

wu-bin commented Apr 24, 2019

/assign @danzh2010

@wu-bin
Copy link
Copy Markdown
Contributor Author

wu-bin commented Apr 25, 2019

/assign @jmarantz

Copy link
Copy Markdown
Contributor

@danzh2010 danzh2010 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just out of curiosity, why do we prefer small test-only libraries instead of one that bundles whatever needed for QUICHE e2e tests? quic_test.h, quic_expect_bug.h and quic_mock_log.h are needed for every test.

@wu-bin
Copy link
Copy Markdown
Contributor Author

wu-bin commented Apr 25, 2019

Just out of curiosity, why do we prefer small test-only libraries instead of one that bundles whatever needed for QUICHE e2e tests? quic_test.h, quic_expect_bug.h and quic_mock_log.h are needed for every test.

They are not needed by every test, but this is not important. My preference for small libraries comes from 1) It's what is done in internal quiche libraries, and 2) it's more flexible, it can express more complex dependency graphs compared to monolithic libraries. (Not saying I like complex dependency graphs:)

Copy link
Copy Markdown
Contributor

@danzh2010 danzh2010 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@danzh2010
Copy link
Copy Markdown
Contributor

/assign @htuch

@wu-bin
Copy link
Copy Markdown
Contributor Author

wu-bin commented Apr 25, 2019

/assign @alyssawilk

@htuch htuch merged commit c7f54a0 into envoyproxy:master Apr 25, 2019
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.

5 participants