Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: disconnect connected buses at end of tests #25

Merged
merged 4 commits into from
Sep 20, 2022
Merged

Conversation

mdegat01
Copy link
Collaborator

@mdegat01 mdegat01 commented Sep 20, 2022

Last build failed for confusing reasons. I suspect from the error the real reason is something not being released between tests. Adding a call to bus.disconnect at the end of every test which connects to the bus seems to fix it.

@codecov
Copy link

codecov bot commented Sep 20, 2022

Codecov Report

Base: 80.88% // Head: 81.19% // Increases project coverage by +0.31% 🎉

Coverage data is based on head (9a744d7) compared to base (cfad28b).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #25      +/-   ##
==========================================
+ Coverage   80.88%   81.19%   +0.31%     
==========================================
  Files          24       24              
  Lines        2835     2835              
  Branches      616      616              
==========================================
+ Hits         2293     2302       +9     
+ Misses        334      327       -7     
+ Partials      208      206       -2     
Impacted Files Coverage Δ
src/dbus_fast/message_bus.py 73.91% <0.00%> (+1.13%) ⬆️
src/dbus_fast/glib/message_bus.py 80.50% <0.00%> (+1.27%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@mdegat01 mdegat01 changed the title ci: empty pr to run ci fix: disconnect connected buses at end of tests Sep 20, 2022
@mdegat01 mdegat01 marked this pull request as ready for review September 20, 2022 20:23
@mdegat01
Copy link
Collaborator Author

This change also seems to have dropped the number of warnings in the pytest log about unexpected disconnects and unclosed resources from ~28 to ~7 (depending on the version). Fwiw there was only one test that connected a bus which I did not change and that was this one:

async def test_tcp_connection_with_forwarding(event_loop):
closables = []
host = "127.0.0.1"
port = "55556"
addr_info = parse_address(os.environ.get("DBUS_SESSION_BUS_ADDRESS"))
assert addr_info
assert "abstract" in addr_info[0][1]
path = f'\0{addr_info[0][1]["abstract"]}'
async def handle_connection(tcp_reader, tcp_writer):
unix_reader, unix_writer = await asyncio.open_unix_connection(path)
closables.append(tcp_writer)
closables.append(unix_writer)
async def handle_read():
while True:
data = await tcp_reader.read(1)
if not data:
break
unix_writer.write(data)
async def handle_write():
while True:
data = await unix_reader.read(1)
if not data:
break
tcp_writer.write(data)
asyncio.run_coroutine_threadsafe(handle_read(), event_loop)
asyncio.run_coroutine_threadsafe(handle_write(), event_loop)
server = await asyncio.start_server(handle_connection, host, port)
closables.append(server)
bus = await MessageBus(bus_address=f"tcp:host={host},port={port}").connect()
# basic tests to see if it works
result = await bus.call(
Message(
destination="org.freedesktop.DBus",
path="/org/freedesktop/DBus",
interface="org.freedesktop.DBus.Peer",
member="Ping",
)
)
assert result
intr = await bus.introspect("org.freedesktop.DBus", "/org/freedesktop/DBus")
obj = bus.get_proxy_object("org.freedesktop.DBus", "/org/freedesktop/DBus", intr)
iface = obj.get_interface("org.freedesktop.DBus.Peer")
await iface.call_ping()
assert bus._sock.getpeername()[0] == host
assert bus._sock.getsockname()[0] == host
assert bus._sock.gettimeout() == 0
assert bus._stream.closed is False
for c in closables:
c.close()

It seemed to be testing what happened when the socket was closed on the bus so I didn't want to change that one around.

@bdraco bdraco merged commit e438890 into main Sep 20, 2022
@bdraco bdraco deleted the fix-3.9-failure branch September 20, 2022 20:28
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