Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: fixing build dependencies #19783

Merged
merged 4 commits into from
Feb 9, 2022
Merged

Conversation

alyssawilk
Copy link
Contributor

Part of #9953

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@repokitteh-read-only
Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #19783 was opened by alyssawilk.

see: more, trace.

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@alyssawilk alyssawilk marked this pull request as ready for review February 3, 2022 13:31
@htuch htuch self-assigned this Feb 6, 2022
htuch
htuch previously approved these changes Feb 8, 2022
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

LGTM, but not sure about the disabled tests.

@@ -429,7 +161,7 @@ TEST_P(VhdsIntegrationTest, RdsUpdateWithoutVHDSChangesDoesNotRestartVHDS) {
// - Upstream makes a request to an (now) unknown domain
// - A VHDS DiscoveryResponse received containing update for the domain
// - Upstream receives a 200 response
TEST_P(VhdsIntegrationTest, VhdsVirtualHostAddUpdateRemove) {
TEST_P(VhdsIntegrationTest, DISABLED_VhdsVirtualHostAddUpdateRemove) {
Copy link
Member

Choose a reason for hiding this comment

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

Why are tests also being disabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

whoops - I moved them over and forgot to delete them from the old test file - ditto the commented out tests

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@htuch htuch merged commit 489807b into envoyproxy:main Feb 9, 2022
joshperry pushed a commit to joshperry/envoy that referenced this pull request Feb 13, 2022
Part of envoyproxy#9953

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Josh Perry <josh.perry@mx.com>
@alyssawilk alyssawilk deleted the move_ondemand branch March 20, 2023 14:37
@alyssawilk alyssawilk restored the move_ondemand branch March 20, 2023 14:40
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.

2 participants