varnishcache-friends / libvmod-geoip2

Varnish VMOD to query MaxMind GeoIP2 DB files
BSD 2-Clause "Simplified" License
39 stars 17 forks source link

Updated the expected MaxMind output. #45

Closed GeertHauwaerts closed 2 years ago

GeertHauwaerts commented 2 years ago

The connection-type for the test IP changed in the MaxMind test database.

fgsch commented 2 years ago

Hi. I cannot reproduce this. How are you testing it?

GeertHauwaerts commented 2 years ago

This b00001.log is the log (without my PR) for test b00001, it fails with the latest MaxMind MMDB.

Verification of the new value:

root@varnish-0485:~# wget -q https://github.com/maxmind/MaxMind-DB/raw/main/test-data/GeoIP2-Connection-Type-Test.mmdb
root@varnish-0485:~# mmdblookup -f GeoIP2-Connection-Type-Test.mmdb -i 1.0.128.1

  {
    "connection_type":
      "Cable/DSL" <utf8_string>
  }

root@varnish-0485:~#
codecov[bot] commented 2 years ago

Codecov Report

Merging #45 (e6672c9) into devel (7e9e56d) will not change coverage. The diff coverage is n/a.

:exclamation: Current head e6672c9 differs from pull request most recent head a3246cf. Consider uploading reports for the commit a3246cf to get more accurate results

@@           Coverage Diff           @@
##            devel      #45   +/-   ##
=======================================
  Coverage   95.32%   95.32%           
=======================================
  Files           1        1           
  Lines         107      107           
=======================================
  Hits          102      102           
  Misses          5        5           

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

fgsch commented 2 years ago

These tests depend on the database used in the libmaxminddb repo under t/maxmind-db, currently at revision 31a33b3. I can see this has changed in a later revision but until the submodule is updated, this shouldn't be needed.

That said, can you explain what you are trying to do?

GeertHauwaerts commented 2 years ago

The SaltStack module we use excluded the submodule, we've fixed it upstream there instead.

fgsch commented 2 years ago

@GeertHauwaerts Cool, thanks for letting me know.