Skip to content

Refactor auth tests to use real communities, remove mocks#255

Merged
neuromechanist merged 2 commits intodevelopfrom
85-refactor-authorization-tests-to-remove-mocks
Mar 9, 2026
Merged

Refactor auth tests to use real communities, remove mocks#255
neuromechanist merged 2 commits intodevelopfrom
85-refactor-authorization-tests-to-remove-mocks

Conversation

@neuromechanist
Copy link
Member

Summary

  • Remove all MagicMock, @patch, and patch.dict from authorization tests
  • Load real community configs via discover_assistants() at module level
  • Use monkeypatch.setenv/delenv for environment variable control (not mocking; real Settings reads them)
  • Add _clear_settings_cache autouse fixture to clear @lru_cache between tests
  • Test against real community CORS origins (HED, MNE, EEGLAB, BIDS)
  • Add test_cross_community_origins_not_shared for isolation verification
  • All 29 original test scenarios preserved and passing

Test plan

  • All 29 auth tests pass
  • Zero mock imports remaining in file
  • Pre-commit hooks pass (ruff lint + format)

Closes #85

Replace MagicMock and @patch with real community configurations
loaded via discover_assistants(). Use monkeypatch for environment
variables (real Settings reads them). All test scenarios preserved.

Closes #85
Derive test origins and model values from loaded community configs
instead of hardcoding them. Move discover_assistants() into a
module-scoped fixture for better error reporting. Add helper
functions for config access.
@neuromechanist
Copy link
Member Author

PR Review

Code review found 2 important issues, both addressed in follow-up commit:

  1. Hardcoded config values violate testing guidelines (confidence: 85) - Values like CORS origins and default_model were hardcoded. Project guidelines say "Dynamic tests: Query registries/configs, don't hardcode values." Fixed by adding helper functions (_get_config, _get_exact_origin, _get_wildcard_origin) that derive test data from loaded community configs.

  2. discover_assistants() at module level (confidence: 80) - Called during test collection, making failures confusing. Fixed by moving to a module-scoped autouse fixture for proper error reporting.

Test coverage review: All critical authorization paths covered (29 tests). Minor gaps identified (empty cors_origins path, community_config=None in select_api_key) are low severity (4-5/10) and the behavior is straightforward in those paths.

@neuromechanist neuromechanist merged commit 5a323f0 into develop Mar 9, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant