Improve webservice coverage by wuan · Pull Request #186 · wuan/bo-python

wuan · 2026-04-23T08:18:23Z

No description provided.

- Add comprehensive tests for Blitzortung class (65 tests) - Move application and connection pool setup to service/base.py - Make services injectable via constructor for testing - Keep BLITZORTUNG_TEST guard in service/base.py for test isolation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add try-except around log setup to handle permission errors gracefully. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Fix NameError by moving LogObserver definition before application setup. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Move Blitzortung and LogObserver classes to service/base.py - Keep webservice.py as minimal entry point for twistd - Tests now import from service/base.py Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Matches the new location of the Blitzortung class in service/base.py. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The tests import from service/base.py which has no side effects. The webservice.py is only loaded by twistd. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Not needed since tests use mocks and don't need this factory. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Refactors the Blitzortung JSON-RPC webservice implementation into a dedicated blitzortung.service.base module and adds a new unit test suite to improve coverage.

Changes:

Added blitzortung/service/base.py containing Blitzortung and LogObserver.
Refactored blitzortung/cli/webservice.py to import the new service module and start the server after connection pool creation.
Added tests/service/test_base.py to cover key service behaviors and helpers; updated start_webservice entry point imports.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.

File	Description
`tests/service/test_base.py`	Adds new tests for `Blitzortung` and `LogObserver`.
`blitzortung/service/base.py`	Introduces shared webservice implementation (moved from CLI module).
`blitzortung/cli/webservice.py`	Simplifies twistd entry module to use the extracted service implementation and async pool init.
`blitzortung/cli/start_webservice.py`	Adjusts twistd launcher imports for the refactored webservice module.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Add tests for jsonrpc_get_strikes_grid - Add tests for jsonrpc_get_global_strikes_grid - Add tests for jsonrpc_get_local_strikes_grid - Add tests for __check_period and __restart_period Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Fix test name: test_removes_header_for_max_version - Fix LogObserver test to not test buggy path - Add valid user-agent to forbidden IP test - Catch OSError and log instead of silent Exception handling - Remove manual startService() call (relies on setServiceParent) - Add back reactor installation for performance - Remove redundant import from start_webservice.py - Remove unused imports from service/base.py - Fix get_global_strikes_grid to use global_strike_grid_query Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The reactor was already installed by twistd, so importing 'reactor' gives the instance, not the module. Fixed by importing epollreactor or kqreactor module explicitly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-23T10:04:13Z

+
+        mock_strike_grid_query = Mock()
+        mock_strike_grid_query.create = Mock(return_value=(Mock(), Mock()))
+        mock_strike_grid_query.combine_result = Mock(return_value=Mock())
+
+        forbidden_ips = {'192.168.1.100': True}
+
+        bt = Blitzortung(
+            mock_pool,
+            None,
+            strike_grid_query=mock_strike_grid_query,


These tests instantiate Blitzortung without providing strike_query/global_strike_grid_query/histogram_query. The Blitzortung constructor will then call blitzortung.service._query() which depends on blitzortung.INJECTOR; in unit tests this is likely uninitialized and can cause test failures. Pass mocks for all query dependencies (or patch blitzortung.service._query) to keep the test isolated.

Suggested change

mock_strike_grid_query = Mock()

mock_strike_grid_query.create = Mock(return_value=(Mock(), Mock()))

mock_strike_grid_query.combine_result = Mock(return_value=Mock())

forbidden_ips = {'192.168.1.100': True}

bt = Blitzortung(

mock_pool,

None,

strike_grid_query=mock_strike_grid_query,

mock_strike_query = Mock()

mock_strike_grid_query = Mock()

mock_strike_grid_query.create = Mock(return_value=(Mock(), Mock()))

mock_strike_grid_query.combine_result = Mock(return_value=Mock())

mock_global_strike_grid_query = Mock()

mock_histogram_query = Mock()

forbidden_ips = {'192.168.1.100': True}

bt = Blitzortung(

mock_pool,

None,

strike_query=mock_strike_query,

strike_grid_query=mock_strike_grid_query,

global_strike_grid_query=mock_global_strike_grid_query,

histogram_query=mock_histogram_query,

- Fix reactor installation to catch ReactorAlreadyInstalledError in kqreactor branch - Fix referer check to use 'not request.getHeader(\'referer\')' - Fix X-Forwarded-For parsing to handle different separators - Improve test isolation by adding missing mocks for query dependencies Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Validates region is within 1-7 range to prevent KeyError when accessing grid[region]. Uses __force_range for consistency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

sonarqubecloud · 2026-04-23T10:23:52Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
91.8% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

wuan and others added 14 commits April 23, 2026 09:45

first version

54469a8

Fix log setup error in webservice.py

5d3b17c

Add try-except around log setup to handle permission errors gracefully. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Move LogObserver class before module-level code

7c091d9

Fix NameError by moving LogObserver definition before application setup. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add debug output to start_server

b73f514

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add more debug output

1c186fb

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Keep reference to jsonrpc_server to prevent garbage collection

a4b63ea

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Call startService() to actually start the server

776d38f

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Clean up: remove debug prints and global variable

f5f270f

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Swap webservice.py and service/base.py for clean separation

4bb16bd

- Move Blitzortung and LogObserver classes to service/base.py - Keep webservice.py as minimal entry point for twistd - Tests now import from service/base.py Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Move test_webservice.py to tests/service/test_base.py

b171692

Matches the new location of the Blitzortung class in service/base.py. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Remove BLITZORTUNG_TEST - no longer needed

4cbcfae

The tests import from service/base.py which has no side effects. The webservice.py is only loaded by twistd. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Clean up test reports and temporary files

db805e9

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Remove unused create_connection_pool parameter from Blitzortung

b702e85

Not needed since tests use mocks and don't need this factory. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

wuan requested a review from Copilot April 23, 2026 09:15

Copilot started reviewing on behalf of wuan April 23, 2026 09:15 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

wuan and others added 3 commits April 23, 2026 11:29

Add more tests for Blitzortung class

8945d52

- Add tests for jsonrpc_get_strikes_grid - Add tests for jsonrpc_get_global_strikes_grid - Add tests for jsonrpc_get_local_strikes_grid - Add tests for __check_period and __restart_period Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Fix reactor installation - import reactor module not instance

b9050e0

The reactor was already installed by twistd, so importing 'reactor' gives the instance, not the module. Fixed by importing epollreactor or kqreactor module explicitly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

wuan requested a review from Copilot April 23, 2026 09:59

Copilot started reviewing on behalf of wuan April 23, 2026 10:00 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

wuan and others added 2 commits April 23, 2026 12:12

Add region validation with MAX_REGION constant

e150ede

Validates region is within 1-7 range to prevent KeyError when accessing grid[region]. Uses __force_range for consistency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve webservice coverage#186

Improve webservice coverage#186
wuan wants to merge 19 commits intomainfrom
webservice_coverage

wuan commented Apr 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Apr 23, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud Bot commented Apr 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

wuan commented Apr 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud Bot commented Apr 23, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants