Skip to content

fix(test): call configure() on script addons in taddons.context.configure - #8369

Open
citizen204 wants to merge 1 commit into
mitmproxy:mainfrom
citizen204:fix-3402-taddons-configure-script-addon
Open

fix(test): call configure() on script addons in taddons.context.configure#8369
citizen204 wants to merge 1 commit into
mitmproxy:mainfrom
citizen204:fix-3402-taddons-configure-script-addon

Conversation

@citizen204

Copy link
Copy Markdown

Summary

context.script() returns an addon that is register()-ed but never added to AddonManager.chain. The options.changed -> _configure_all -> trigger() broadcast that normally calls configure() after an option change only walks self.chain, so a script addon's configure() is silently never called again after the initial load — even though context.configure(addon, some_option=...) updates the option value itself just fine. This makes it impossible to write a test that exercises a script addon's configure() reaction to an option change.

Reproduced on current main with a minimal script addon that records its configure() calls: after tctx.configure(addon, foo='bar'), the option updates but configure() is never invoked a second time.

Changes

  • mitmproxy/test/taddons.py: context.configure() now explicitly invokes ConfigureHook on the target addon after updating options, but only when that addon isn't already part of self.master.addons.chain — chained addons (the common case, e.g. context(some_addon)) already receive the hook via the existing options.changed broadcast, so this avoids calling configure() twice for them.
  • test/mitmproxy/data/addonscripts/configure_recorder.py: new fixture script addon that records every updated set passed to its configure().
  • test/mitmproxy/test_taddons.py: regression test loading the fixture via context.script() and asserting configure() is called with the changed option key after context.configure(). Verified the test fails on unpatched taddons.py and passes with the fix.

Fixes #3402

…gure

context.script() returns an addon that is registered but not part of the
addon chain, so the options.changed broadcast that normally triggers
configure() for chained addons never reaches it. context.configure()
now invokes configure() on it directly after updating options.

Fixes mitmproxy#3402
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.

tests: taddons tctx.configure does not call configure on addon

1 participant