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

Ci fixes and updates #454

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions .github/workflows/cd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ on:
ref:
description: "Enter a tag or commit to package"
default: ""
gvsbuild-tag:
description: "Use an alternative gvsbuild release for the windows build. Defaults to latest."
default: "latest"

jobs:
windows_package:
Expand All @@ -24,8 +27,8 @@ jobs:
strategy:
matrix:
arch: [x64, x86]
python: ["3.9"]
libtorrent: [2.0.7, 1.2.19]
python: ["3.7", "3.10"]
libtorrent: [2.0.8, 1.2.19]

steps:
# Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it
Expand All @@ -49,12 +52,20 @@ jobs:
cache: pip

- name: Prepare pip
run: python -m pip install wheel setuptools==68.*
run: python -m pip install wheel setuptools==70.*

- name: Determine gvsbuild release URL
id: gvsbuild-url
shell: bash
run: |
test -z "${{ github.event.inputs.gvsbuild-tag }}" && tag=latest || tag="${{ github.event.inputs.gvsbuild-tag }}"
if [[ "$tag" == "latest" ]]; then URL="https://github.com/${{ github.repository_owner }}/gvsbuild-release/releases/$tag/download"; else URL="https://github.com/${{ github.repository_owner }}/gvsbuild-release/releases/download/$tag" ; fi
echo "gvsbuild-release-url=$URL" >> $GITHUB_OUTPUT

- name: Install GTK
run: |
$WebClient = New-Object System.Net.WebClient
$WebClient.DownloadFile("https://github.com/deluge-torrent/gvsbuild-release/releases/download/latest/gvsbuild-py${{ matrix.python }}-vs16-${{ matrix.arch }}.zip","C:\GTK.zip")
$WebClient.DownloadFile("${{ steps.gvsbuild-url.outputs.gvsbuild-release-url }}/gvsbuild-py${{ matrix.python-version }}-vs17-${{matrix.arch}}.zip","C:\GTK.zip")
7z x C:\GTK.zip -oc:\GTK
echo "C:\GTK\release\lib" | Out-File -FilePath $env:GITHUB_PATH -Append
echo "C:\GTK\release\bin" | Out-File -FilePath $env:GITHUB_PATH -Append
Expand All @@ -77,7 +88,6 @@ jobs:
working-directory: deluge_src
run: |
python -m pip install .
python setup.py install_scripts

- name: Freeze Deluge
working-directory: packaging/win
Expand Down
34 changes: 30 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,19 @@ on:
core-dump:
description: "Set to 1 to enable retrieving core dump from crashes"
default: "0"
gvsbuild-tag:
description: "Use an alternative gvsbuild release for the windows build. Defaults to latest."
default: "latest"
jobs:
test-linux:
runs-on: ubuntu-22.04
strategy:
matrix:
python-version: ["3.7", "3.10"]

python-version: ["3.9", "3.10"]
os: ["ubuntu-24.04"]
include:
- os: ubuntu-22.04
python-version: 3.7
runs-on: ${{ matrix.os }}
steps:
# Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it
- uses: actions/checkout@v4
Expand All @@ -36,6 +42,7 @@ jobs:

- name: Install dependencies
run: |
sudo apt-get install libcairo2-dev libgirepository1.0-dev
pip install --upgrade pip wheel setuptools
pip install -r requirements-ci.txt
pip install -e .
Expand Down Expand Up @@ -74,7 +81,8 @@ jobs:
runs-on: windows-2022
strategy:
matrix:
python-version: ["3.7", "3.10"]
python-version: ["3.7", "3.9", "3.10"]
arch: ["x64"]

steps:
# Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it
Expand All @@ -89,6 +97,24 @@ jobs:
cache: "pip"
cache-dependency-path: "requirements*.txt"

- name: Determine gvsbuild release URL
id: gvsbuild-url
shell: bash
run: |
test -z "${{ github.event.inputs.gvsbuild-tag }}" && tag=latest || tag="${{ github.event.inputs.gvsbuild-tag }}"
if [[ "$tag" == "latest" ]]; then URL="https://github.com/${{ github.repository_owner }}/gvsbuild-release/releases/$tag/download"; else URL="https://github.com/${{ github.repository_owner }}/gvsbuild-release/releases/download/$tag" ; fi
echo "gvsbuild-release-url=$URL" >> $GITHUB_OUTPUT

- name: Install GTK
run: |
$WebClient = New-Object System.Net.WebClient
$WebClient.DownloadFile("${{ steps.gvsbuild-url.outputs.gvsbuild-release-url }}/gvsbuild-py${{ matrix.python-version }}-vs17-${{matrix.arch}}.zip","C:\GTK.zip")
7z x C:\GTK.zip -oc:\GTK
echo "C:\GTK\release\lib" | Out-File -FilePath $env:GITHUB_PATH -Append
echo "C:\GTK\release\bin" | Out-File -FilePath $env:GITHUB_PATH -Append
echo "C:\GTK\release" | Out-File -FilePath $env:GITHUB_PATH -Append
python -m pip install --no-index --find-links="C:\GTK\release\python" pycairo PyGObject

- name: Install dependencies
run: |
pip install --upgrade pip wheel setuptools
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/docs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ jobs:
run: |
pip install --upgrade pip wheel
pip install tox
sudo apt-get install enchant-2
sudo apt-get install enchant-2 libgirepository1.0-dev

- name: Build docs with tox
env:
Expand Down
5 changes: 3 additions & 2 deletions deluge/tests/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import traceback

import pytest
import pytest_twisted
from twisted.internet import defer, protocol, reactor
from twisted.internet.defer import Deferred
from twisted.internet.error import CannotListenError
Expand Down Expand Up @@ -130,7 +131,7 @@ def outConnectionLost(self): # NOQA: N802
with open(self.logfile, 'w') as f:
f.write(self.log_output)

@defer.inlineCallbacks
@pytest_twisted.inlineCallbacks
def kill(self):
"""Kill the running process.

Expand Down Expand Up @@ -296,7 +297,7 @@ def start_core(
if extra_callbacks:
callbacks.extend(extra_callbacks)

@defer.inlineCallbacks
@pytest_twisted.inlineCallbacks
def shutdown_daemon():
username, password = get_localhost_auth()
client = Client()
Expand Down
4 changes: 2 additions & 2 deletions deluge/tests/daemon_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#

import pytest
from twisted.internet import defer
import pytest_twisted
from twisted.internet.error import CannotListenError

import deluge.component as component
Expand All @@ -29,7 +29,7 @@ def terminate_core(self, *args):
d = self.core.kill()
return d

@defer.inlineCallbacks
@pytest_twisted.inlineCallbacks
def start_core(
self,
arg,
Expand Down
10 changes: 5 additions & 5 deletions deluge/tests/test_maybe_coroutine.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,19 @@
import pytest
import pytest_twisted
import twisted.python.failure
from twisted.internet import defer, reactor, task
from twisted.internet import reactor, task
from twisted.internet.defer import maybeDeferred

from deluge.decorators import maybe_coroutine


@defer.inlineCallbacks
@pytest_twisted.inlineCallbacks
def inline_func():
result = yield task.deferLater(reactor, 0, lambda: 'function_result')
return result


@defer.inlineCallbacks
@pytest_twisted.inlineCallbacks
def inline_error():
raise Exception('function_error')
yield
Expand All @@ -35,13 +35,13 @@ async def coro_error():
raise Exception('function_error')


@defer.inlineCallbacks
@pytest_twisted.inlineCallbacks
def coro_func_from_inline():
result = yield coro_func()
return result


@defer.inlineCallbacks
@pytest_twisted.inlineCallbacks
def coro_error_from_inline():
result = yield coro_error()
return result
Expand Down
16 changes: 10 additions & 6 deletions deluge/tests/test_ui_entry.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

import pytest
import pytest_twisted
from twisted.internet import defer

import deluge
import deluge.component as component
Expand Down Expand Up @@ -353,7 +352,8 @@ def test_console_command_add(self):
fd = StringFileDescriptor(sys.stdout)
self.patch(sys, 'stdout', fd)

yield self.exec_command()
with mock.patch('deluge.ui.console.main.ConsoleUI.quit', autospec=True):
yield self.exec_command()

std_output = fd.out.getvalue()
assert (
Expand All @@ -377,7 +377,8 @@ def test_console_command_add_move_completed(self):
fd = StringFileDescriptor(sys.stdout)
self.patch(sys, 'stdout', fd)

yield self.exec_command()
with mock.patch('deluge.ui.console.main.ConsoleUI.quit', autospec=True):
yield self.exec_command()

std_output = fd.out.getvalue()
assert std_output.endswith(
Expand All @@ -391,19 +392,22 @@ async def test_console_command_status(self):
self.patch_arg_command(['status'])
self.patch(sys, 'stdout', fd)

await self.exec_command()
with mock.patch('deluge.ui.console.main.ConsoleUI.quit', autospec=True):
await self.exec_command()

std_output = fd.out.getvalue()
assert std_output.startswith('Total upload: ')
assert std_output.endswith(' Moving: 0\n')

@defer.inlineCallbacks
@pytest_twisted.inlineCallbacks
def test_console_command_config_set_download_location(self):
fd = StringFileDescriptor(sys.stdout)
self.patch_arg_command(['config --set download_location /downloads'])
self.patch(sys, 'stdout', fd)

yield self.exec_command()
with mock.patch('deluge.ui.console.main.ConsoleUI.quit', autospec=True):
Copy link
Contributor Author

@Lord-Kamina Lord-Kamina Sep 2, 2024

Choose a reason for hiding this comment

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

This was calling the real ConsoleUI.quit(), which was stopping the reactor and thus making the tests go boom.

yield self.exec_command()

std_output = fd.out.getvalue()
assert std_output.startswith('Setting "download_location" to: \'/downloads\'')
assert std_output.endswith('Configuration value successfully updated.\n')
Expand Down
6 changes: 3 additions & 3 deletions deluge/tests/test_web_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

import pytest
import pytest_twisted
from twisted.internet import defer, reactor
from twisted.internet import reactor
from twisted.web.client import Agent, FileBodyProducer
from twisted.web.http_headers import Headers
from twisted.web.static import File
Expand Down Expand Up @@ -45,7 +45,7 @@ def on_connect(result):
def test_disconnect(self):
d = self.deluge_web.web_api.connect(self.host_id)

@defer.inlineCallbacks
@pytest_twisted.inlineCallbacks
def on_connect(result):
assert self.deluge_web.web_api.connected()
yield self.deluge_web.web_api.disconnect()
Expand Down Expand Up @@ -76,7 +76,7 @@ def test_set_config(self):
assert config['pwd_sha1'] != web_config['pwd_sha1']
assert config['sessions'] != web_config['sessions']

@defer.inlineCallbacks
@pytest_twisted.inlineCallbacks
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These changes are due to the pytest developers recommending that twisted not be used directly in tests with fixtures. From the pytest-twisted docs:

Using twisted.internet.defer.inlineCallbacks as a decorator for test functions, which use fixtures, does not work. Please use pytest_twisted.inlineCallbacks instead.

def get_host_status(self):
host = list(self.deluge_web.web_api.hostlist.get_host_info(self.host_id))
host[3] = 'Online'
Expand Down
4 changes: 2 additions & 2 deletions deluge/ui/gtk3/gtkui.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@
# isort:imports-thirdparty
from gi.repository.GLib import set_prgname
from gi.repository.Gtk import Builder, ResponseType
from twisted.internet import defer, gtk3reactor
from twisted.internet import defer, gireactor
from twisted.internet.error import ReactorAlreadyInstalledError
from twisted.internet.task import LoopingCall

try:
# Install twisted reactor, before any other modules import reactor.
reactor = gtk3reactor.install()
reactor = gireactor.install()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just to avoid using a deprecated class name.

Copy link
Member

Choose a reason for hiding this comment

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

We support a minimum version of Twisted and afaik this would break that so needs to be reverted, I don't think they are actually removing it anytime soon. We could add a Twisted version check for the import but would need a comment to explain why gireactor is being used since the Twisted docs are now contradictory about not using it for UI.

Copy link
Contributor Author

@Lord-Kamina Lord-Kamina Sep 4, 2024

Choose a reason for hiding this comment

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

Does it though? the current requirement spec is:

twisted[tls]>=17.1; sys_platform != 'win32'
twisted[tls]<23,>=17.1; sys_platform == 'win32'

And the Twisted api documentation for 16.4.1, a full major version earlier (https://twisted.org/documents/16.4.1/api/twisted.internet.gireactor.html), already has the gireactor, with an API that I think is identical?

except ReactorAlreadyInstalledError:
# Running unit tests so already installed a rector
from twisted.internet import reactor
Expand Down
2 changes: 1 addition & 1 deletion requirements-ci.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
-r requirements.txt
-r requirements-tests.txt
pytest
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, pytest is pinned due to console tests failing, which is fixed by this PR.

libtorrent==2.0.7
pytest==7.4.2
2 changes: 1 addition & 1 deletion requirements-tests.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
libtorrent
libtorrent==2.0.7
pytest
pytest-twisted
pytest-cov
Expand Down
Loading