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

Remove DownloadManager.dummy_mode #7797

Closed
qstokkink opened this issue Jan 4, 2024 · 3 comments
Closed

Remove DownloadManager.dummy_mode #7797

qstokkink opened this issue Jan 4, 2024 · 3 comments

Comments

@qstokkink
Copy link
Contributor

In #7466 it was uncovered that dummy_mode of DownloadManager is uncovered.

The dummy_mode code being uncovered suggests that the corresponding code is dead code and can be removed. However, we should check that this is indeed the case.

If the code related to dummy_mode is indeed dead code, we can remove it.

@drew2a
Copy link
Contributor

drew2a commented Jan 4, 2024

How the dummy_mode is used in our test infrastructure

The dummy_mode is equivalent to the gui_test_mode.

The gui_test_mode is used in test_gui.py while performing GUI tests

core_args=[str(RUN_TRIBLER_PY.absolute()), '--core', '--gui-test-mode'],

Therefore it is used in guitest workflow: https://github.com/Tribler/tribler/blob/main/.github/workflows/guitest.yml

What is the dummy_mode

The following lines are affected by dummy_mode:

self.dht_readiness_timeout = self.config.dht_readiness_timeout if not self.dummy_mode else 0

# If libtorrent session has pending disk io, wait until timeout (default: 30 seconds) to let it finish.
# In between ask for session stats to check if state is clean for shutdown.
# In dummy mode, we immediately shut down the download manager.
while not self.dummy_mode and not self.is_shutdown_ready() and timeout >= 1:
self.notify_shutdown_state("Waiting for LibTorrent to finish...")
self.post_session_stats()
timeout -= 1
await asyncio.sleep(1)

if self.dummy_mode:
from unittest.mock import Mock
ltsession = Mock()
ltsession.pop_alerts = lambda: {}
ltsession.listen_port = lambda: 123
ltsession.get_settings = lambda: {"peer_fingerprint": "000"}
else:
ltsession = lt.session(lt.fingerprint(*fingerprint), flags=0) if hops == 0 else lt.session(flags=0)

if self.config.dht and not self.dummy_mode:
ltsession.start_dht()
for router in DEFAULT_DHT_ROUTERS:
ltsession.add_dht_router(*router)
ltsession.start_lsd()

if not self.dummy_mode:
self._logger.info('Dummy mode is disabled. Starting handle.')
await self.start_handle(download, atp)

self.checkpoint_disabled = checkpoint_disabled or self.dummy

@qstokkink
Copy link
Contributor Author

Working from the results of this analysis, I think dummy_mode should be removed because it is only used for tests. My reasoning is that the system under test is changing its behavior to fit the tests. Instead, the tests should configure the system to test certain behavior.

@qstokkink qstokkink changed the title Check potential removal of DownloadManager.dummy_mode Remove DownloadManager.dummy_mode Jan 4, 2024
@qstokkink
Copy link
Contributor Author

This appears to have been resolved in #7466

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

No branches or pull requests

2 participants