Skip to content

[testing] Provider overrides, teardown and mocks in NitroTestingModule - #102

Open
ROHIT-JR wants to merge 5 commits into
nitrocloudofficial:developfrom
ROHIT-JR:feat/74-testing-overrides
Open

ROHIT-JR wants to merge 5 commits into
nitrocloudofficial:developfrom
ROHIT-JR:feat/74-testing-overrides

Conversation

@ROHIT-JR

Copy link
Copy Markdown
Collaborator

Closes #74

What this adds

  • NitroTestingModule.create(AppModule, overrides={Token: instance_or_factory}). An instance is used as is; a class or plain function is called with no arguments (it may be async). The overrides are registered before the app is built, so the original provider is never constructed.
  • await harness.close() and async with. close() calls each provider's on_module_destroy hook (sync or async) once, in reverse creation order, then removes the overrides from the DI container so the next test starts clean. It is idempotent, and every hook runs even if one fails (the first error is raised afterwards). Overrides are also undone if create() itself fails.
  • MockLogger (has_log(level, message), clear()), create_mock_context(**overrides) and flush_tasks(), all in nitrostack.testing.
  • Optional pytest fixture nitro_client in nitrostack.testing.pytest_plugin, selected with @pytest.mark.nitro_app(module=Root) or a nitro_app fixture, and closed after the test. This replaces the old PR feat: expand security, background tasks, and dev tooling #4 fixture; the marker takes module= because pytest treats a lone class passed to a marker as a decoration.
  • README testing example. tests/test_readme_examples.py runs every README block marked <!-- readme-test -->.

Notes

  • The SDK had no provider shutdown hook, so this defines one: a method named on_module_destroy. Say if you prefer a different name.
  • McpApplication now keeps _module_instances (used by close()) and skips unittest.mock objects when it scans providers for tools. A MagicMock answers hasattr for everything, so without this it was registered as a tool.

Tests

tests/test_testing_module_overrides.py (15 tests) and tests/test_readme_examples.py (2): override used and original never built, factory forms, close once when called twice, async with, overrides restored (also after a failed create), failing hook, MagicMock overrides, logger, context, flush_tasks, the fixture, and no warnings with -W error. Full suite: 841 passed locally.

@jpsiddharth2008 jpsiddharth2008 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed a7422cb. Please address these before merging:

  1. Harness isolation: close() restores override tokens but leaves dependent services cached. A second harness can reuse a closed service holding the previous fake.
  2. Teardown: Track all resolved dependencies in creation order. The current module-instance list misses indirect dependencies and can destroy dependencies before their consumers.
  3. Factory failure rollback: Include override factory evaluation in the rollback block. A failing factory currently leaves earlier overrides installed.
  4. Pytest fixture: Resolve nitro_app dynamically. Checking request.fixturenames rejects a defined fixture when the test requests only nitro_client.

I reproduced all four issues independently. Existing focused tests and CI pass, but these cases need regression coverage.

@ROHIT-JR

ROHIT-JR commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks for reproducing these. All four are fixed in 2715212, each with a regression test that fails on the old code:

  1. Isolation: create() snapshots the DI container and close() restores it, so services built during the harness (and the fakes they hold) are gone. A second harness gets fresh ones.
  2. Teardown: hooks now run over every instance the harness built, found through the container, so indirect dependencies are included. They run in reverse creation order, so consumers are destroyed before their dependencies.
  3. Factory failure: override evaluation is inside the rollback block, so a failing factory leaves nothing installed.
  4. Fixture: nitro_app is resolved with request.getfixturevalue, so a test that requests only nitro_client works.

Full suite: 845 passed locally. I dropped the _module_instances attribute from app.py since it is no longer needed.

@jpsiddharth2008
jpsiddharth2008 self-requested a review November 2, 2026 12:47

@jpsiddharth2008 jpsiddharth2008 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks, the four earlier findings are fixed. One related case remains: if Service is already cached before create(..., overrides={Repo: fake}), it retains the real Repo, so the tool ignores the override.

Please rebuild affected consumers within the harness while preserving the original instances for restoration on close, and add a regression test for this case.

@ROHIT-JR

ROHIT-JR commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks, good catch. Fixed in c1babfc.

create() now evicts any cached consumer of an overridden token, and anything that depends on it (found through each class's declared deps), so the container rebuilds them inside the harness with the override. The snapshot taken first puts the original instances back on close(), and they are the same objects, untouched (their shutdown hooks do not run).

Two regression tests, both failing on the previous commit: a consumer resolved before create() now gets the fake and is restored by identity on close, and the same for an indirect consumer. Full suite: 847 passed locally.

…iginals untouched

close() found instances to destroy by token, so a consumer rebuilt by create()
(same token as the original) was skipped. Compare by object identity instead.
Tests now check override, shutdown of rebuilt services and restoration of the
originals together, for direct and indirect consumers.
@ROHIT-JR

ROHIT-JR commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 1f880c8, which fixes the teardown gap.

Problem: after create() rebuilt a consumer that was cached before the override, close() skipped it. close() picked instances to destroy by token, and the rebuilt service has the same token as the original.

Fix: close() now compares by object identity. It runs on_module_destroy on every object this harness built, and on nothing that existed before. The original instances are still restored by identity and are never shut down.

Tests: the direct and indirect consumer tests now check three things together:

  1. The override reaches the rebuilt service.
  2. The rebuilt services and the override are each shut down once (indirect case order: Outer, Service, then the fake).
  3. The originals come back as the same objects with no shutdown run.

Both tests fail on c1babfc and pass now. Full suite passes (847) and CI is green on 3.10, 3.11 and 3.12.

This branch has not been deployed

No deployments
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.

2 participants