Repository navigation
Conversation
| servers = [] | ||
| node.stash[SERVERS_STASH_KEY] = servers | ||
| if server not in servers: | ||
| servers.append(server) |
There was a problem hiding this comment.
I think this will leak :)
There was a problem hiding this comment.
yup, The stash entry is never removed, and pytest keeps items in session.items, so for a function-scoped custom server (the BlockingHTTPServer example) the stopped server, its log and handlers stay referenced until the session ends — harmless for the plugin's own session-scoped servers.
| "Note: these errors were not raised because the test did not call the check " | ||
| "methods of the server (for example HTTPServer.check())." | ||
| ) | ||
| report.sections.append((REPORT_SECTION_TITLE, "\n".join(section_lines))) |
There was a problem hiding this comment.
Found while testing: when a test fails in both call and teardown, this section gets appended twice — once to each failed report. It doesn't happen on the last test of a session, because the session-scoped server is cleared before that teardown report is built, which makes it easy to miss.
Deleting the stash right after this append (and at the teardown report when there's nothing to report) fixes it and releases the references
There was a problem hiding this comment.
So I asked my agent again to fix this (right now I still consider it as a proof of concept), it claims it is fixed and there's a test added for this.
I'm still unsure if this is the right implementation, perhaps I'll take a closer look later. I need to be more familiar with pytest semantics and hooks, this current implementation seems overcomplicated for my first sight. :)
Thanks for keeping feedback on this.
Once this code is cleaned up (and most importantly: understood by me) I think this thing will be first put behind a feature flag (eg. envvar or such), and it will be opt-in.
Zsolt
2 participants
No description provided.