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 tests on some platforms #124

Merged
merged 4 commits into from
Oct 29, 2021
Merged
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
10 changes: 5 additions & 5 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
fail-fast: false
matrix:
os: [ubuntu-latest]
python-version: [3.6, 3.7, 3.8, 3.9, "3.10.0"]
python-version: [3.6, 3.7, 3.8, 3.9, "3.10"]
env: [{ MINIMAL: "true" }, { MINIMAL: "false" }]
include:
# custom tests
Expand Down Expand Up @@ -58,10 +58,10 @@ jobs:
restore-keys: |
${{ runner.os }}-pip-

# optional, just for certain versions
- name: Install LXML dependencies
if: ${{ matrix.python-version == '3.10.0' || matrix.python-version == 'pypy3' }}
run: sudo apt-get install libxml2-dev libxslt-dev
# optional, just for certain versions where prebuilt wheels don't exist
- name: Install LXML and pycld3 dependencies
if: ${{ matrix.python-version == '3.10' || matrix.python-version == 'pypy3' }}
run: sudo apt-get install libxml2-dev libxslt-dev protobuf-compiler

# package setup
- uses: actions/checkout@v2
Expand Down
17 changes: 14 additions & 3 deletions tests/cli_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import io
import logging
import os
import subprocess
import sys

from collections import deque
Expand Down Expand Up @@ -79,12 +80,18 @@ def test_parser():

def test_climain():
'''test arguments and main CLI entrypoint'''
assert os.system('trafilatura --help') % 256 == 0 # exit status
if os.name == 'nt':
trafilatura_bin = os.path.join(sys.prefix, "Scripts", "trafilatura")
else:
trafilatura_bin = os.path.join(sys.prefix, "bin", "trafilatura")

assert subprocess.run([trafilatura_bin, '--help']).returncode == 0 # exit status
## doesn't pass remote tests, 256 or 0 is OK
# piped input
assert os.system('echo "<html><body></body></html>" | trafilatura') % 256 == 0
empty_input = b'<html><body></body></html>'
assert subprocess.run([trafilatura_bin], input=empty_input).returncode == 0
# input directory walking and processing
assert os.system('trafilatura --inputdir "tests/resources/"') % 256 == 0
result = subprocess.run([trafilatura_bin, '--inputdir', RESOURCES_DIR]).returncode == 0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops I just realized that I removed the assert here @adbar
I initially wanted to assert on result but I changed my mind and didn't add the assert back

Copy link
Owner

Choose a reason for hiding this comment

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

should be fixed in 07216a3

Copy link
Owner

Choose a reason for hiding this comment

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

error on Windows for the line that wasn't part of the tests...

Copy link
Contributor Author

@vbarbaresi vbarbaresi Oct 29, 2021

Choose a reason for hiding this comment

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

oh right, it's the same issue that I fixed in the multiprocess pool workers.. So adding PYTHONIOENCODING in the run command environment should fix the issue:

subprocess.run([...], env={"PYTHONIOENCODING": "utf-8"})

But I'll try on a Windows machine this week-end.
I want to see if it should be set globally in the app or if it's just a test issue on GitHub worker configuration.
I suspect we have to fix it globally and not just in the tests

Copy link
Owner

Choose a reason for hiding this comment

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

ok!



def test_input_type():
Expand Down Expand Up @@ -195,6 +202,10 @@ def test_cli_pipeline():
# cli.process_args(args)
#assert len(f.getvalue()) == 0
# test URL listing

# Force encoding to utf-8 for Windows in future processes spawned by multiprocessing.Pool
os.environ['PYTHONIOENCODING'] = "utf-8"

testargs = ['', '--list']
with patch.object(sys, 'argv', testargs):
args = cli.parse_args(testargs)
Expand Down