logstash-plugins / logstash-filter-geoip

Apache License 2.0
64 stars 82 forks source link

ci: refactor specs with useful boundary from LS-core extension #220

Closed yaauie closed 1 year ago

yaauie commented 1 year ago

The existing specs reach into the internals of the Geoip Database Manager that may be provided by LS core, but those details are changing with Logstash 8.12's introduction of a Geoip Database Manager as a core feature.

Here we make a number of changes to port the existing specs to a form that validates the same desired behaviour without needing to reach into the implementation of the LS-core-provided extension.

  1. we can trust that if LogStash::Filters::GeoIP::DatabaseManager is available:
    • it will be a singleton that responds to subscribe_database_path and unsubscribe_database_path, and behaves as-specified in Logstash core, so we can write our spec in terms of those two methods only
    • it and its Extension will be loaded before an instance of this plugin can be instantiated, and therefore we should side-load the extension directly before running tests
  2. we extend the specs to cover the OSS-only use-case, and expand the test matrix to cover OSS releases (depends on https://github.com/logstash-plugins/.ci/pull/56)

NOTE: I've captured the output of the specs as-run on LS 7.x, 8.x SNAPSHOT=false (old manager), 8.x SNAPSHOT=true (after elastic/logstash#15348), etc:

https://gist.github.com/yaauie/fa042e9e22d08ad2a2c9f279f41f505e

Things to look for: