Skip to content

Disable quic_platform_test by default. Take 2.#5838

Merged
htuch merged 1 commit intoenvoyproxy:masterfrom
wu-bin:nobuildquiche2
Feb 5, 2019
Merged

Disable quic_platform_test by default. Take 2.#5838
htuch merged 1 commit intoenvoyproxy:masterfrom
wu-bin:nobuildquiche2

Conversation

@wu-bin
Copy link
Contributor

@wu-bin wu-bin commented Feb 4, 2019

Description:

Disable quic_platform_test by default. Enable it only when "--define quiche=enabled" is specified in the bazel command line.

This is needed to workaround a issue with --experimental_remap_main_repo + ci asan build, see here for details.

Risk Level: minimal: build only
Testing:

I introduced a failure in quic_platform_test, then confirmed quic_platform_test still passes by default(since it does nothing), and failed when "--define enable_quiche=enabled" is specified.

Docs Changes: none
Release Notes: none
[Optional Fixes #Issue]
[Optional Deprecated:]

Signed-off-by: Bin Wu <wub@google.com>
@wu-bin
Copy link
Contributor Author

wu-bin commented Feb 4, 2019

/assign @mpwarres

@htuch htuch self-assigned this Feb 5, 2019
Copy link
Contributor

@mpwarres mpwarres left a comment

Choose a reason for hiding this comment

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

LGTM

Sidenote: it appears that in copying the description from #5791 to this PR, the link to the original issue got stripped. Here it is, just for context: #5767 (comment)

Copy link
Contributor

@alyssawilk alyssawilk left a comment

Choose a reason for hiding this comment

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

cool, thanks for exempting this by default

unfortunately I'm, having desktop access isssues so lazy question - do we have this on for at least one of the CI builds?

@wu-bin
Copy link
Contributor Author

wu-bin commented Feb 5, 2019

cool, thanks for exempting this by default

unfortunately I'm, having desktop access isssues so lazy question - do we have this on for at least one of the CI builds?

No, I think we want to disable it in CI builds before this issue is fixed. WDYT?

@htuch htuch merged commit e2bf272 into envoyproxy:master Feb 5, 2019
fredlas pushed a commit to fredlas/envoy that referenced this pull request Mar 5, 2019
Disable quic_platform_test by default. Enable it only when "--define quiche=enabled" is specified in the bazel command line.

This is needed to workaround a issue with --experimental_remap_main_repo + ci asan build, see here for details.

Risk Level: minimal: build only
Testing: I introduced a failure in quic_platform_test, then confirmed quic_platform_test still passes by default(since it does nothing), and failed when "--define enable_quiche=enabled" is specified.

Signed-off-by: Bin Wu <wub@google.com>
Signed-off-by: Fred Douglas <fredlas@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