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

Failing tests for v3.3.0 on Alpine Linux #139

Closed
OTLabs opened this issue Feb 14, 2024 · 8 comments · Fixed by #166
Closed

Failing tests for v3.3.0 on Alpine Linux #139

OTLabs opened this issue Feb 14, 2024 · 8 comments · Fixed by #166
Assignees
Labels
bug Something isn't working
Milestone

Comments

@OTLabs
Copy link

OTLabs commented Feb 14, 2024

I am building updated package for v3.3.0 and giving followup to all your kind feedback about failing tests.

Now I got following tests failing:

=================================== FAILURES ===================================
__________________________ test_selocale_unsupported ___________________________

tmp_path = PosixPath('/tmp/pytest-of-builder/pytest-0/test_selocale_unsupported0')

    def test_selocale_unsupported(tmp_path):
>       with pytest.raises(locale.Error):
E       Failed: DID NOT RAISE <class 'locale.Error'>

tests/i18n/test_i18n.py:27: Failed
__________________ test_ogvjs_installed_script_missing_param ___________________

    def test_ogvjs_installed_script_missing_param():
        # run from installed script to check real conditions
        script = subprocess.run(
            ["/usr/bin/env", "fix_ogvjs_dist"],
            text=True,
            capture_output=True,
            check=False,
        )
>       assert script.returncode == 1
E       assert 127 == 1
E        +  where 127 = CompletedProcess(args=['/usr/bin/env', 'fix_ogvjs_dist'], returncode=127, stdout='', stderr="env: can't execute 'fix_ogvjs_dist': No such file or directory\n").returncode

tests/ogvjs/test_ogvjs.py:55: AssertionError
________________________ test_ogvjs_installed_script_ok ________________________

tmp_path = PosixPath('/tmp/pytest-of-builder/pytest-0/test_ogvjs_installed_script_ok0')
videojs_url = 'https:/videojs/video.js/releases/download/v7.6.4/video-js-7.6.4.zip'
ogvjs_url = 'https:/brion/ogv.js/releases/download/1.6.1/ogvjs-1.6.1.zip'
videojs_ogvjs_url = 'https:/hartman/videojs-ogvjs/archive/v1.3.1.zip'

    @pytest.mark.slow
    def test_ogvjs_installed_script_ok(tmp_path, videojs_url, ogvjs_url, videojs_ogvjs_url):
        # run from installed script to check real conditions
    
        prepare_ogvjs_folder(tmp_path, videojs_url, ogvjs_url, videojs_ogvjs_url)
    
        script = subprocess.run(
            ["/usr/bin/env", "fix_ogvjs_dist", str(tmp_path)],
            text=True,
            capture_output=True,
            check=False,
        )
>       assert script.returncode == 0
E       assert 127 == 0
E        +  where 127 = CompletedProcess(args=['/usr/bin/env', 'fix_ogvjs_dist', '/tmp/pytest-of-builder/pytest-0/test_ogvjs_installed_script_ok0'], returncode=127, stdout='', stderr="env: can't execute 'fix_ogvjs_dist': No such file or directory\n").returncode

tests/ogvjs/test_ogvjs.py:77: AssertionError



...



=========================== short test summary info ============================
FAILED tests/i18n/test_i18n.py::test_selocale_unsupported - Failed: DID NOT RAISE <class 'locale.Error'>
FAILED tests/ogvjs/test_ogvjs.py::test_ogvjs_installed_script_missing_param - assert 127 == 1
FAILED tests/ogvjs/test_ogvjs.py::test_ogvjs_installed_script_ok - assert 127 == 0
=================== 3 failed, 367 passed in 80.17s (0:01:20) ===================

The first test as expected fails, so I will continue to ignoring it.

Please, let me know if you need a full check log for ogvjs related errors.

Currently I build the wheel and:

python3 -m venv --clear --without-pip --system-site-packages .testenv   
.testenv/bin/python3 -m installer .dist/*.whl
.testenv/bin/python3 -m pytest \
                --runslow \
                --cov=zimscraperlib \
                --cov-report=term --cov-report term-missing
@kelson42 kelson42 added the bug Something isn't working label Feb 15, 2024
@kelson42 kelson42 added this to the 3.4.0 milestone Feb 15, 2024
@kelson42
Copy link
Contributor

@rgaudin @benoit74 Is that a kind of regression? A bit concerned that @OTLabs seems to keep discovering compilation/testing issues post-release. Should we do something to harden our CI?

@rgaudin
Copy link
Member

rgaudin commented Feb 15, 2024

We don't test on Alpine on purpose because we don't support it. We have a blocker (and a ticket) on i18n.

@rgaudin
Copy link
Member

rgaudin commented Feb 15, 2024

@OTLabs I suppose you're following APKBUILD_examples:Python.

Could you share the grep517-built wheel?

The issue is that the tests can't find the generated script in the PATH. Looking at what you run, that seems normal 😅
We should use sys.executable in the test instead of looking for PATH although we were doing it on purpose because we want to make sure this gets installed properly. The solution might be to make this test optional as for --runslow.

Please send the wheel so we can test. I've checked already that grep517 and installer both support entry_points so your not reliance on pip shouldn't be an issue.

@OTLabs
Copy link
Author

OTLabs commented Feb 15, 2024

Here it goes!
zimscraperlib-3.3.0-py3-none-any.whl

@OTLabs
Copy link
Author

OTLabs commented Feb 15, 2024

I would like to share with you the APKBUILD I use to build the package.

@benoit74
Copy link
Collaborator

@rgaudin Maybe we should just restore the --runinstalled flag we removed because we thought we would always be "installed", I think it would be enough?

@benoit74
Copy link
Collaborator

We don't test on Alpine on purpose because we don't support it. We have a blocker (and a ticket) on i18n.

Here it is even worse, I exceptionally ran tests on Alpine (since there was an issue on this) but not with the same procedure than the one use by @OTLabs (I relied on pip to install stuff).

I don't think that having full support of Alpine (and all Linux distro in fact, and all build chains) is a priority for scrapers, please speak up otherwise.

@rgaudin
Copy link
Member

rgaudin commented Feb 16, 2024

@rgaudin Maybe we should just restore the --runinstalled flag we removed because we thought we would always be "installed", I think it would be enough?

Yep

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants