Skip to content

quiche: add mem_slice_test and time_utils_test targets#21013

Merged
RyanTheOptimist merged 1 commit intoenvoyproxy:mainfrom
bencebeky:mem-slice-test
Apr 26, 2022
Merged

quiche: add mem_slice_test and time_utils_test targets#21013
RyanTheOptimist merged 1 commit intoenvoyproxy:mainfrom
bencebeky:mem-slice-test

Conversation

@bencebeky
Copy link
Copy Markdown
Contributor

Add quiche_common_mem_slice_test and quiche_common_time_utils_test build
targets of envoy_cc_test kind. Previously quiche_mem_slice_test.cc and
quiche_time_utils_test.cc were part of quiche_common_platform_test,
which is a envoy_cc_test_library (as opposed to a envoy_cc_test),
therefore these tests were not actually ran.

I verified that no other envoy_cc_test_library rules in this
build file have source files matching ".*_test.cc" using the following
command:

bazel query 'attr("srcs", "_test.cc",
attr(generator_function, envoy_cc_test_library,
@com_github_google_quiche//...))'

Signed-off-by: Bence Béky bnc@google.com

Commit Message: Add mem_slice_test and time_utils_test targets.
Additional Description:
Risk Level: low (test-only)
Testing: n/a (test-only)
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

Add quiche_common_mem_slice_test and quiche_common_time_utils_test build
targets of envoy_cc_test kind.  Previously quiche_mem_slice_test.cc and
quiche_time_utils_test.cc were part of quiche_common_platform_test,
which is a envoy_cc_test_library (as opposed to a envoy_cc_test),
therefore these tests were not actually ran.

I verified that no other envoy_cc_test_library rules in this
build file have source files matching ".*_test.cc" using the following
command:

  bazel query 'attr("srcs", "_test.cc", \
      attr(generator_function, envoy_cc_test_library, \
      @com_github_google_quiche//...))'

Signed-off-by: Bence Béky <bnc@google.com>
@bencebeky
Copy link
Copy Markdown
Contributor Author

/assign @danzh2010

Dan: PTAL. I noticed this bug when I tried to run QuicheMemSliceImplTests with the command bazel test @com_github_google_quiche//:quiche_common_platform_test, but no tests were actually ran.

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 over all. A nit about using "quiche: add mem_slice_test and time_utils_test targets" as the title.

@danzh2010
Copy link
Copy Markdown
Contributor

/retest

@repokitteh-read-only
Copy link
Copy Markdown

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #21013 (comment) was created by @danzh2010.

see: more, trace.

@bencebeky bencebeky changed the title Add mem_slice_test and time_utils_test targets. quiche: add mem_slice_test and time_utils_test targets Apr 26, 2022
@bencebeky
Copy link
Copy Markdown
Contributor Author

/assign @RyanTheOptimist

@RyanTheOptimist RyanTheOptimist merged commit a52a88f into envoyproxy:main Apr 26, 2022
@bencebeky bencebeky deleted the mem-slice-test branch April 26, 2022 18:19
ravenblackx pushed a commit to ravenblackx/envoy that referenced this pull request Jun 8, 2022
Add quiche_common_mem_slice_test and quiche_common_time_utils_test build
targets of envoy_cc_test kind.  Previously quiche_mem_slice_test.cc and
quiche_time_utils_test.cc were part of quiche_common_platform_test,
which is a envoy_cc_test_library (as opposed to a envoy_cc_test),
therefore these tests were not actually ran.

I verified that no other envoy_cc_test_library rules in this
build file have source files matching ".*_test.cc" using the following
command:

  bazel query 'attr("srcs", "_test.cc", \
      attr(generator_function, envoy_cc_test_library, \
      @com_github_google_quiche//...))'

Signed-off-by: Bence Béky <bnc@google.com>
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.

4 participants