openzim / libzim

Reference implementation of the ZIM specification
https://download.openzim.org/release/libzim/
GNU General Public License v2.0
164 stars 50 forks source link

Add support to recent googletest framework #830

Closed kelson42 closed 10 months ago

kelson42 commented 10 months ago

Fixes #829

The questions left (I see at this time) are:

codecov[bot] commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (e3316a4) 57.57% compared to head (419b06f) 57.57%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #830 +/- ## ======================================= Coverage 57.57% 57.57% ======================================= Files 98 98 Lines 4545 4545 Branches 1911 1911 ======================================= Hits 2617 2617 Misses 668 668 Partials 1260 1260 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mgautierfr commented 10 months ago

...TEST_CASE is deprecated since gtest 1.10.0 which was released in 2019. We had 5 releases of gtest since (especially 1.13.0 for which we had to move to c++14/17).

Does that not impair compilaation on older distributions (in particular focat)

Focal has gtest 1.10.0 so we are good here.

Should we update subproject/gtest

Yes. (And also add a minimum version in meson.build)

kelson42 commented 10 months ago

@mgautierfr Good, if OK for you, please just fix the last points and merge.

kelson42 commented 10 months ago

@mgautierfr @legoktm Package libgtest-dev has version 1.8.1-3 on Debian Buster, see https://packages.debian.org/source/buster/googletest. But Buster Backports provides version 1.10.0 (which we need) see https://packages.debian.org/source/buster-backports/googletest. @legoktm There is no way you can also add the backport repository in the your Debian Buster Action?

kelson42 commented 10 months ago

@mgautierfr Without feedback from @legoktm I propose to just comment-out or remove buster from the CI.

legoktm commented 10 months ago

Sorry for missing the ping earlier - let's just drop buster support entirely, it's no longer supported by Debian officially, just as part of Debian LTS.

kelson42 commented 10 months ago

@mgautierfr I don't know why we have two linux CI failing because of libicu. Do you know? Can we merge?