Skip to content

deps: bump up googletest to v1.17.0 #39301

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

Closed
wants to merge 2 commits into from

Conversation

agrawroh
Copy link
Contributor

@agrawroh agrawroh commented May 1, 2025

Description

This PR bumps up the version of googletest to v1.17.0


Commit Message: deps: bump up googletest to v1.17.0
Additional Description: Bump up the version of googletest to v1.17.0
Risk Level: Low
Testing: CI
Docs Changes: N/A
Release Notes: N/A

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: #39301 was opened by agrawroh.

see: more, trace.

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label May 1, 2025
Copy link

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @phlax

🐱

Caused by: #39301 was opened by agrawroh.

see: more, trace.

@phlax
Copy link
Member

phlax commented May 1, 2025

theres been a few attempts at updating this one iirc - cc @asedeno (iirc you looked at this last)

@agrawroh agrawroh marked this pull request as ready for review May 1, 2025 16:08
@agrawroh
Copy link
Contributor Author

agrawroh commented May 1, 2025

/retest

@agrawroh
Copy link
Contributor Author

agrawroh commented May 1, 2025

/retest

1 similar comment
@agrawroh
Copy link
Contributor Author

agrawroh commented May 1, 2025

/retest

@asedeno
Copy link
Contributor

asedeno commented May 1, 2025

theres been a few attempts at updating this one iirc - cc @asedeno (iirc you looked at this last)

Yeah, #36699 was my attempt, and some builds kept running out of memory when linking.

@asedeno
Copy link
Contributor

asedeno commented May 1, 2025

@agrawroh this PR is missing many changes I found I needed when attempting to update GoogleTest; look over #36699 to find some useful commits and shortcircuit figuring out what's broken via CI. I tried to keep each commit scoped to a particualr kind of change and with meaningful commit messages to make them easier to understand.

@yanavlasov
Copy link
Contributor

I have cloned your PR in #39305 trying to help with the build. I think clang build may be ok, will see if gcc is happy. It may need to have some test files broken up into smaller pieces.

@agrawroh agrawroh force-pushed the deps-gtest branch 2 times, most recently from 983da0b to 6dc2b06 Compare May 2, 2025 16:01
@agrawroh agrawroh marked this pull request as draft May 2, 2025 16:02
@agrawroh agrawroh force-pushed the deps-gtest branch 2 times, most recently from ff2dc5f to 41b06ef Compare May 3, 2025 14:19
@@ -90,7 +92,7 @@ int TestRunner::runTests(int argc, char** argv) {

// Use the recommended, but not default, "threadsafe" style for the Death Tests.
// See: https://github.com/google/googletest/commit/84ec2e0365d791e4ebc7ec249f09078fb5ab6caa
::testing::FLAGS_gtest_death_test_style = "threadsafe";
GTEST_FLAG_SET(death_test_style, "threadsafe");
Copy link
Contributor

Choose a reason for hiding this comment

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

You can move this before listener is created and avoid the need to change listener.

@agrawroh agrawroh force-pushed the deps-gtest branch 3 times, most recently from 4063e2f to edffd9c Compare May 3, 2025 16:07
Copy link

CC @envoyproxy/coverage-shephards: FYI only for changes made to (test/per_file_coverage.sh).
envoyproxy/coverage-shephards assignee is @RyanTheOptimist
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @abeyad
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #39301 was synchronize by agrawroh.

see: more, trace.

@agrawroh agrawroh force-pushed the deps-gtest branch 2 times, most recently from 7a96e62 to da68e21 Compare May 3, 2025 16:19
@agrawroh agrawroh force-pushed the deps-gtest branch 2 times, most recently from fef21da to b2f60a6 Compare May 4, 2025 07:48
Signed-off-by: Rohit Agrawal <[email protected]>
@agrawroh
Copy link
Contributor Author

agrawroh commented May 5, 2025

Closing this in favor of @yanavlasov's PR here which is clean and have all checks passing!

@agrawroh agrawroh closed this May 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api deps Approval required for changes to Envoy's external dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants