Skip to content

Add test to reproduce Context._pop() AttributeError with capture_hooks enabled#1306

Open
Copilot wants to merge 3 commits intomainfrom
copilot/add-test-for-cleanup-error
Open

Add test to reproduce Context._pop() AttributeError with capture_hooks enabled#1306
Copilot wants to merge 3 commits intomainfrom
copilot/add-test-for-cleanup-error

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jan 20, 2026 •
edited
Loading

PR #1297 fixes a bug where Context._pop() uses self.config instead of self._config at line 336. This test reproduces that bug.

Changes

  • New test class: TestContextCleanupWithCapture in tests/unit/test_context_cleanups.py

    • test_context_pop_with_capture_hooks_enabled: Reproduces the AttributeError when capture_hooks is enabled and config is not in the context stack
    • test_context_pop_with_capture_hooks_and_normal_config_access: Sanity check for normal operation

  • Analysis document: CLEANUP_ERROR_ANALYSIS.md documents the bug, test coverage gap, and verification

Why existing tests missed this

No existing test enabled capture_hooks configuration, so line 336 in runner.py was never executed during testing.

Code analysis

Only one occurrence of this pattern exists in the Context class:

# Line 336 - Bug: uses stack lookup
if self.config.should_capture_hooks():

# Line 340, 393 - Correct: uses direct attribute
with capture_output_to_sink(self._config, ...):

Test behavior

Before fix: Fails with explicit message about the bug
After fix: Passes (verifies PR #1297 works correctly)

Original prompt

This section details on the original issue you should resolve

<issue_title>Reproduce cleanup_error on Context (add test)</issue_title>
<issue_description>PR #1297 attempts to fix a bug in the Context's cleanup logic in Behave's runner module.

@copilot:

  • Find out whether there is a test that covers that error case and why the tests doesn't trigger.
  • If there is no test for this bug yet add one that can clearly reproduce the problem.
  • Don't fix the bug! We only want to supply the test to be integrated into Fixed incorrect config reference in Context causing cleanup_error's #1297.
  • Analyze all self.config references in the module. Which one could be similar errors? Are all occurrences covered by tests?</issue_description>

<agent_instructions>Devise a test that can be merged into PR #1297 proving that the error exists for real. If there are existing tests related to the module find out why the error case was not covered and whether the case can be cleanly integrated. Try to stay concise, not verbose.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Add test to reproduce cleanup_error in Context module Add test to reproduce Context._pop() AttributeError with capture_hooks enabled Jan 20, 2026
Copilot AI requested a review from bittner January 20, 2026 07:43
@bittner bittner marked this pull request as ready for review January 20, 2026 21:23
@bittner bittner mentioned this pull request Jan 20, 2026
Open
@bittner
Copy link
Copy Markdown
Member

bittner commented Jan 20, 2026

Note

We should not merge this PR, but instead have the test integrated in #1297.

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

Reviewers

@bittner bittner Awaiting requested review from bittner

Assignees

@bittner bittner

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Reproduce cleanup_error on Context (add test)

2 participants

@bittner