Conversation
PiperOrigin-RevId: 334384310
…be_unintialized PiperOrigin-RevId: 334391149
Update comment to suggest using SetUpTestSuite and TearDownTestSuite. PiperOrigin-RevId: 334430329
Fix undefined pointer comparison PiperOrigin-RevId: 334436975
Update faq.md on underscore to mention `DISABLED_` prefix. PiperOrigin-RevId: 334507963
…k from A1, A2, ... to TArg1, TArg2,... to avoid clash with legacy header files
PiperOrigin-RevId: 335653055
PiperOrigin-RevId: 336087297
Make the code Python3 compliant. PiperOrigin-RevId: 336144198
Improve lookup of operator<< for user types
Without this fix, trying to use this class with googletest
struct Foo {};
template <typename OutputStream>
OutputStream& operator<<(OutputStream& os, const Foo&) {
os << "TemplatedStreamableInFoo";
return os;
}
results in an ambiguity error between the class' operator<< and the
operator<< in gtest-printers.h removed in this CL.
This fix also enables implicit conversions to happen, so that e.g.
we will find the base class operator<< if a subclass has no
operator<< of its own.
PiperOrigin-RevId: 336261221
Add helper methos to internal FlatTuple. Refactor constructors. PiperOrigin-RevId: 336306681
Suggest using generic lambdas for composing macros.
Long chains of macros hurt legibility; generic lambdas are an easy way to abbreviate them, but are not an obvious solution to casual users.
Compare:
EXPECT_THAT(f(), ElementsAre(
Property(&MyClass::foo, Property(&OtherClass::bar, Contains("x"))),
Property(&MyClass::foo, Property(&OtherClass::bar, Contains("y"))));
to:
EXPECT_THAT(f(), ElementsAre(HasFooBar("x"), HasFooBar("y")));
PiperOrigin-RevId: 336870137
Use absl::StrCat in MATCHER_P example for consistency with https://abseil.io/tips/3 PiperOrigin-RevId: 336878481
PiperOrigin-RevId: 336881266
Fixes AppVeyor by upgrading to Bazel 3.6.0 PiperOrigin-RevId: 336887434
Fix -Wmismatched-tags error with struct tuple_size vs class tuple_size PiperOrigin-RevId: 336930166
Prefer using over typedef. PiperOrigin-RevId: 337080404
Disable -Wmismatched-tags warning for struct/class tuple_size PiperOrigin-RevId: 337087493
Removing a semicolon that triggers a lint error in sample code. PiperOrigin-RevId: 337095451
Stop using master.zip to make the build reproducible PiperOrigin-RevId: 337102716
Disable warnings on code that intentionally tests a suboptimal syntax PiperOrigin-RevId: 337138442
Add ::testing::FieldsAre matcher for objects that support get<> and structured bindings. PiperOrigin-RevId: 337165285
…plate-args-AX-to-TArgX PiperOrigin-RevId: 337217118
Fixes build warnings from previous CL Add CMake to internal presubmit to prevent these PiperOrigin-RevId: 337325504
Fix some issues when running fuse_gmock_files. The module path should be updated before importing `fuse_gtest_files`, since the script may not run from the googletest repo root. We also need a non-frozen set in order to track progress. PiperOrigin-RevId: 337380466
Fix typo in the "Assertion Placement" section PiperOrigin-RevId: 337435223
Replace "definedin in" by "defined in" in files: - googletest/src/gtest.cc - googletest/test/googletest-output-test-golden-lin.txt
|
CI just to see if things are happy with this:
|
|
Another try:
|
ChrisThrasher
left a comment
There was a problem hiding this comment.
This updating is holding back MoveIt2 from using -Wpedantic. I can confirm that this works and lets us finally enable that warning.
|
Unfortunately it is still causing CI to be yellow, and I haven't had time to go back and look at why. If you have some time to look at that and propose possible fixes, that would be helpful. |
|
https://ci.ros2.org/job/ci_linux/17846/gcc/new/source.3769323b-695d-4413-a06b-0a8173357ff9/#316 The error seems to be that there's some new code in v1.11 that triggers |
You are right, nowadays we are treating them as system headers, so this probably isn't a problem anymore. That said, this change is not compiling anymore locally for me. I had to upgrade a bunch of our packages to C++17 (which we honestly should have done a while ago). I'm slowly getting those PRs in, but this will take some time. Once we have those in, I'll rebase this and try CI again. |
Did this update accidentally start using C++17 features? The latest release only claims to require C++14. What specifically fails to compile for you? |
When I go to compile this PR against the rest of the ROS 2 core, |
|
https://github.com/osrf/osrf_testing_tools_cpp/blob/master/osrf_testing_tools_cpp/CMakeLists.txt#L9 |
|
OK, we've now updated osrf_testing_tools_cpp to use C++17. Let's give CI another shot here:
|
cottsay
left a comment
There was a problem hiding this comment.
A few things:
- This needs a rebase or merge to take changes to the package manifest from
rolling - The
CMakeLists.txt.upstreamshould be updated within thegoogletestandgooglemocksubdirectories - this appears to have been lost in the merge. - The version snapshot list in the package manifests immediately prior to the version number should be updated with the new google/googletest ref
|
I'm not sure why, but GitHub failed to back-link from the other PR. This is the bug fix in |
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
|
I think I've now fixed all of the issues pointed out by @cottsay. With the other fixes that are in place, let's see how CI does now:
|
|
All right, with ros2/rclcpp#2227 now merged, I'm going to mark this PR as ready for review and run another CI on it. |
|
CI:
|
cottsay
left a comment
There was a problem hiding this comment.
I reviewed the diff and it aligns with my attempt to do the same.
Thanks!
|
CI is green, and this is approved, so merging. Note that I am going to create a merge commit here. |
This matches the version that is in Ubuntu 22.04, and it should fix some warnings when building with Clang.
Draft for now while I get some feedback on it.