google / s2geometry

Computational geometry and spatial indexing on the sphere
http://s2geometry.io/
Apache License 2.0
2.29k stars 302 forks source link

Document that googletest version 1.10.0 is required #259

Closed MikePlayle closed 1 year ago

MikePlayle commented 2 years ago

With revision 7a40135059545396237a0199c558d749fe3be0b1 I get this build failure:

/home/mike/devel/github/google/s2geometry/src/s2/s2shape_nesting_query_test.cc:452:25: error: expected constructor, destructor, or type conversion before ‘(’ token
 INSTANTIATE_TEST_SUITE_P(
                         ^
/home/mike/devel/github/google/s2geometry/src/s2/s2shape_nesting_query_test.cc:470:6: error: expected unqualified-id before ‘)’ token
     }));
      ^

It was introduced by change 0df6f8cf9d3d7671ab8943430ffc4eac5ae20123, which added s2shape_nesting_query_test.cc.

I can't find any other references to INSTANTIATE_TEST_SUITE_P. Removing it gets me a successful build, but am I removing test coverage?

I think somebody who understands this macro should advise on how to fix this.

jmr commented 2 years ago

Do you see it in your gtest sources? What gtest version are you using?

MikePlayle commented 2 years ago

1.8.1, which turned out to be too old. With 1.10.0 it works fine. Whoops.

Perhaps the README should mention this version requirement, but if you want to call this user error, I'm happy with that too.

jmr commented 2 years ago

Yes, we should specify a minimum version.

For some reason we say macports should use 1.8.0. I'm actually using 1.11.0 now.

jmr commented 2 years ago

Looks like 1.10.0 should be enough. The macro was added in Jan 2019, and that's the next release.

https://github.com/google/googletest/commit/3a460a26b7a91abf87af7f31b93d29f930e25c82 https://github.com/google/googletest/releases