Opened 3 years ago

Closed 2 years ago

#34305 closed Cleanup/optimization (fixed)

Refactor the GeoIP2 tests to use MaxMind test data

Reported by: Jacob Walls Owned by: Nick Pope
Component: GIS Version: dev
Severity: Normal Keywords: geoip, geoip2, maxmind, maxmind-db, mmdb
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Attempted to run the gis_tests today. Ran into the django-docker-box ​issue that depends on a refactor in Django itself, so opening a ticket here to track that work.

Essentially, Tom Forbes proposes committing MaxMind test data to the django repo to enable the test_geoip2 to run without having to download the city and country test files. Looking at the PR, a small refactor was also necessary to enable using @override_settings to point to the files.

​PR

Change History (6)

comment:1 by Claude Paroz, 3 years ago

Triage Stage: Unreviewed → Accepted

Looks good! Please refer to this ticket number in the commit message.

comment:2 by Mariusz Felisiak, 3 years ago

Patch needs improvement: set

Both PRs are no longer mergeable.

comment:3 by Nick Pope, 2 years ago

Keywords: geoip geoip2 maxmind maxmind-db mmdb added
Owner: changed from Tom Forbes to Nick Pope
Patch needs improvement: unset

​PR

comment:4 by Mariusz Felisiak, 2 years ago

Triage Stage: Accepted → Ready for checkin

comment:5 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

In ed4f837:

Refs #34305 -- Added SimpleTestCase.enterClassContext() on Python < 3.11.

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

Resolution: → fixed
Status: assigned → closed

In a93375e8:

Fixed #34305 -- Changed GeoIP2 tests to use MaxMind test databases.

GEOIP_SETTINGS is removed from the global scope as this prevents
modifications to the settings using @override_settings in tests.

Additional improvements now that we have stable test databases include:

  • Made testing more comprehensive and improved coverage
  • Patched socket.gethostbyname() for whole test case
  • Added testing of non-free GeoIP2 databases

Co-authored-by: Tom Forbes <tom@…>

Note: See TracTickets for help on using tickets.
Back to Top