Skip to content

Tests: geoip2 http & stream tests - #107

Open
nitin9977 wants to merge 1 commit into
nginx:masterfrom
nitin9977:nitin/geoip2-migration
Open

Tests: geoip2 http & stream tests#107
nitin9977 wants to merge 1 commit into
nginx:masterfrom
nitin9977:nitin/geoip2-migration

Conversation

@nitin9977

@nitin9977 nitin9977 commented Aug 20, 2026

Copy link
Copy Markdown

Proposed changes

Testcases for ngx_http_geoip2_module and ngx_stream_geoip2_module backed by the maintained libmaxminddb.
Tests PR: nginx/nginx#1655

Checklist

Before creating a PR, run through this checklist and mark each as complete:

@nitin9977
nitin9977 force-pushed the nitin/geoip2-migration branch from 8e51466 to c7acc07 Compare August 20, 2026 12:05
@nitin9977 nitin9977 self-assigned this Aug 20, 2026
@sindhushiv sindhushiv moved this from New to In Review in NGINX OSS Unified Workspace Aug 20, 2026

@jimf5 jimf5 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is worth adding IPv6 tests upon readiness.

Comment thread geoip2.t
my $mmdb_dir = $ENV{TEST_NGINX_MMDB_DIR}
// './geoip';

plan(skip_all => 'GeoIP2 test MMDB databases not found')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We typically create all required dependencies within the test and do not store sample data in the repository.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jimf5 Yes, I saw the same for existing GeoIP testcases, but I want to discuss this with wider audience. My thought process is that we should not be creating the db files, instead use the official test db files(~50KB).

Will bring this in one of our dev sync-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

3 participants