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.

Sign up for GitHub

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

Jump to bottom

test: fixing build dependencies #19783

Merged
merged 4 commits into from
Feb 9, 2022
Merged

test: fixing build dependencies #19783

merged 4 commits into from
Feb 9, 2022

Conversation

alyssawilk
Copy link
Contributor

Part of #9953

@alyssawilk
test: fixing build dependencies
465dd09 
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.

@alyssawilk
vhds
e824edb 
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@alyssawilk
spelling
810eb63 
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@alyssawilk alyssawilk marked this pull request as ready for review February 3, 2022 13:31
@alyssawilk alyssawilk requested a review from htuch as a code owner February 3, 2022 13:31
@htuch htuch self-assigned this Feb 6, 2022
htuch
htuch previously approved these changes Feb 8, 2022
View reviewed changes
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.

test/integration/vhds_integration_test.cc Outdated
@@ -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

@alyssawilk
Cleanup
70c2b17 
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
@alyssawilk
test: fixing build dependencies (envoyproxy#19783)
19a9970 
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
Reviewers

@htuch htuch htuch approved these changes

Assignees

@htuch htuch

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@alyssawilk @htuch