Skip to content

Fix xfail tests - "args" attribute in ArchitectCoder#349

Merged
dwash96 merged 4 commits intocecli-dev:v0.92.0from
johbo:fix-xfail-tests
Jan 2, 2026
Merged

Fix xfail tests - "args" attribute in ArchitectCoder#349
dwash96 merged 4 commits intocecli-dev:v0.92.0from
johbo:fix-xfail-tests

Conversation

@johbo
Copy link
Copy Markdown

@johbo johbo commented Jan 1, 2026

Fixing two tests of the AgentCoder to work again.

After looking into the code a bit deeper I got the idea that the tests should avoid to mock the __init__ method. It seems that creating the coder object in the regular way works, this should make the tests depend a lot less on the details of the init method.

johbo added 2 commits January 1, 2026 23:42
Add defensive check for self.args which may not exist when
ArchitectCoder is instantiated without full initialization.
Use getattr to safely access the attribute and default to None.
- Remove xfail markers from 3 ArchitectCoder tests
- Use AsyncMock for async generate() method
- Fix assertions to include allow_tweak parameter
- Handle SwitchCoder exception raised by reply_completed()
- Add missing coder attributes (aider_commit_hashes, etc.)
- Remove boilerplate comments
@johbo johbo marked this pull request as draft January 1, 2026 23:05
Comment thread aider/coders/architect_coder.py Outdated
return

tweak_responses = getattr(self.args, "tweak_responses", False)
args = getattr(self, "args", None)
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I wonder if args shoud always be there on an instance of a coder, so that the getattr trickery here could be avoided.

Copy link
Copy Markdown
Collaborator

@dwash96 dwash96 Jan 1, 2026

Choose a reason for hiding this comment

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

It's not really that you need to do trickery with getattr(), it's that the actual create function in base_coder should make sure anything that has a relevant from_coder passed to it should get the args (the object we parse from the configuration all the way at the beginning of main.py main_async method) is just passed down to children.

We already do this with the tui weakref and mcp_servers

Edit: and the args object itself already after checking!

Copy link
Copy Markdown
Collaborator

@dwash96 dwash96 Jan 1, 2026

Choose a reason for hiding this comment

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

However, most tests don't instantiate an args object at all and that's the real problem with the harness. It's not a true state of affairs in the actual application that a coder will exist without an args object being passed into it first from main.py and then through SwitchCoder exceptions, by way of the from_coder argument

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

did arrive there eventually, think the test case was mocking the init method and making it difficult for itself.

johbo added 2 commits January 2, 2026 00:22
Tests patch AskCoder.__init__ which bypasses normal initialization.
Fix the tests to properly set coder.args rather than adding defensive
code to production that handles a test-only scenario.
Replace manual __init__ patching with proper Coder.create() factory.
This reduces test maintenance burden by letting the coder initialize
normally instead of manually setting 10+ attributes.
@johbo johbo marked this pull request as ready for review January 2, 2026 00:04
@dwash96 dwash96 merged commit 3124091 into cecli-dev:v0.92.0 Jan 2, 2026
1 check passed
@dwash96 dwash96 mentioned this pull request Jan 2, 2026
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