flathub / org.qutebrowser.qutebrowser

https://flathub.org/apps/details/org.qutebrowser.qutebrowser
5 stars 8 forks source link

enable tests #18

Closed tinywrkb closed 2 years ago

tinywrkb commented 3 years ago

Running the test might help to catch up packaging or Flatpak specific issues before distributed the built app.

I've done some initial crude work to try and get the tests running, directly executing pytest in order to try and avoid the multiple pyenv set by tox for Python version we don't care about.
We don't have networking during build time which is another reason why we want to avoid the default tox config.

Currently, the tests are failing with // edit: nvm, already fixed

___________________________________ ERROR collecting tests/end2end/test_mkvenv.py ____________________________________
ImportError while importing test module '/run/build/qutebrowser/tests/end2end/test_mkvenv.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/end2end/test_mkvenv.py:22: in <module>
    from scripts import mkvenv
E   ModuleNotFoundError: No module named 'scripts'
_______________________________ ERROR collecting tests/unit/config/test_configutils.py _______________________________
ImportError while importing test module '/run/build/qutebrowser/tests/unit/config/test_configutils.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/unit/config/test_configutils.py:28: in <module>
    from tests.helpers import testutils
E   ModuleNotFoundError: No module named 'tests'
________________________________ ERROR collecting tests/unit/scripts/test_dictcli.py _________________________________
ImportError while importing test module '/run/build/qutebrowser/tests/unit/scripts/test_dictcli.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/unit/scripts/test_dictcli.py:28: in <module>
    from scripts import dictcli
E   ImportError: cannot import name 'dictcli' from 'scripts' (unknown location)
________________________________ ERROR collecting tests/unit/scripts/test_importer.py ________________________________
ImportError while importing test module '/run/build/qutebrowser/tests/unit/scripts/test_importer.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/unit/scripts/test_importer.py:23: in <module>
    from scripts import importer
E   ImportError: cannot import name 'importer' from 'scripts' (unknown location)
____________________________ ERROR collecting tests/unit/scripts/test_problemmatchers.py _____________________________
ImportError while importing test module '/run/build/qutebrowser/tests/unit/scripts/test_problemmatchers.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/unit/scripts/test_problemmatchers.py:24: in <module>
    from scripts.dev.ci import problemmatchers
E   ModuleNotFoundError: No module named 'scripts.dev'
collected 9105 items / 5 errors / 7 skipped / 9093 selected
tinywrkb commented 3 years ago

Closing as the tests depend on a display that I don't believe is available during the build process.

The-Compiler commented 3 years ago

If you have pytest-xvfb (and Xvfb itself) installed, there's no display needed. As for the issues above, setting PYTHONPATH=. should help.

tinywrkb commented 3 years ago

Thanks, it looks like I'm advancing a little forward now that I have pytest-xvfb installed.
I'm still probably missing other modules as I encountered more errors but the main blocker might be the networking requirement.
I'll look at this again at a later date, the current failed tests are probably my fault.

p.s. I forgot to mention that the tests packaging update is in my WIP tests branch.

The-Compiler commented 3 years ago

In theory, the tests should work without network (other than the HTTP server on localhost running as part of the tests). Can I find the logs somewhere?

tinywrkb commented 3 years ago

Here's the build log without networking enabled: qutebrowser_tests.log

The-Compiler commented 3 years ago

Relevant parts:


    def _random_port(self) -> int:
        """Get a random free port."""
        sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
>       sock.bind(('localhost', 0))
E       socket.gaierror: [Errno -3] Temporary failure in name resolution

That looks like even networking on localhost doesn't work. Can you try adding an early "return 1234" in that function and see if that makes those tests work? If it's just the resolution of the "localhost" name, I guess there's a way around it.

Maybe also try '127.0.0.1' in place of 'localhost' there?


tests/unit/browser/test_caret.py sFatal Python error: Aborted

Current thread 0x00007f5bbf942740 (most recent call first):
  File "/run/build/qutebrowser/tests/helpers/fixtures.py", line 183 in testdata_scheme

Looks like QtWebEngine called abort() while initializing. If you run pytest with -v -s --no-qt-log you should hopefully get more information.

tinywrkb commented 3 years ago

Thanks! switching to an IP address resolved the networking requirement.

I'm not sure if the following is the only issue but it seems to affect multiple tests.

15:01:31.540 DEBUG    init       app:run:86 Main process PID: 98
15:01:31.540 DEBUG    init       app:run:88 Initializing directories...
15:01:31.540 DEBUG    init       standarddir:init:345 Base directory: /tmp/qutebrowser-basedir-fez30lsb
INVALID: Traceback (most recent call last):
INVALID:   File "/usr/lib/python3.8/runpy.py", line 194, in _run_module_as_main
INVALID:     return _run_code(code, main_globals, None,
INVALID:   File "/usr/lib/python3.8/runpy.py", line 87, in _run_code
INVALID:     exec(code, run_globals)
INVALID:   File "/run/build/qutebrowser/qutebrowser/__main__.py", line 29, in <module>
INVALID:     sys.exit(qutebrowser.qutebrowser.main())
INVALID:   File "/run/build/qutebrowser/qutebrowser/qutebrowser.py", line 218, in main
INVALID:     return app.run(args)
INVALID:   File "/run/build/qutebrowser/qutebrowser/app.py", line 89, in run
INVALID:     standarddir.init(args)
INVALID:   File "/run/build/qutebrowser/qutebrowser/utils/standarddir.py", line 347, in init
INVALID:     _init_dirs(args)
INVALID:   File "/run/build/qutebrowser/qutebrowser/utils/standarddir.py", line 338, in _init_dirs
INVALID:     _init_runtime(args)
INVALID:   File "/run/build/qutebrowser/qutebrowser/utils/standarddir.py", line 238, in _init_runtime
INVALID:     assert app_name == APPNAME, app_name
INVALID: AssertionError: runtime
FAILED

I'm uploading a partial log as the full one is a bit large.
qutebrowser_tests_20210319_1500.log

The-Compiler commented 3 years ago

Well, turns out enabling tests was already worth it, because there was a bug in https://github.com/qutebrowser/qutebrowser/commit/9f67a763ef86805f0981f037ccd2fb5cb0e84b88 after all... You can reproduce manually by starting with --temp-basedir.

Here's the fix: https://github.com/qutebrowser/qutebrowser/commit/2f8fbff53a34adfbf4fe3e295de795fd4135ddcb

I forgot to split the tests from the commit, so you might need to edit the patch to remove those, as they were added separatly from the original commit.

tinywrkb commented 3 years ago

The QtWebkit backend tests fail.

15:07:36.032 DEBUG    init       app:_init_modules:454 Initializing logging from config...
15:07:36.032 DEBUG    init       log:init_from_config:543 --debug flag overrides log configs
15:07:36.032 DEBUG    init       app:_init_modules:458 Initializing save manager...
15:07:36.033 DEBUG    init       app:_init_modules:464 Checking backend requirements...
15:07:36.033 ERROR    init       backendproblem:_show_dialog:176 <b>Failed to start with the QtWebKit backend!</b><p>qutebrowser tried to start with the QtWebKit backend but failed because QtWebKit could not be imported.</p><p><b>The error encountered was:</b><br/>cannot import name &#x27;QtWebKit&#x27; from &#x27;PyQt5&#x27; (/app/lib/python3.8/site-packages/PyQt5/__init__.py)</p><p><b>Forcing the QtWebEngine backend</b></p><p>This forces usage of the QtWebEngine backend by setting the <i>backend = 'webengine'</i> option (if you have a <i>config.py</i> file, you'll need to set this manually). </p>
15:07:36.035 DEBUG    destroy    objreg:on_destroyed:125 schedule removal: crash-handler
15:07:36.035 DEBUG    destroy    objreg:on_destroyed:125 schedule removal: save-manager
FAILED

Log: qutebrowser_20210324_1507.log

Using --qute-bdd-webengine makes this error disappear but I'm not sure that it's the correct way to disable webkit.

 18:45:10.216 DEBUG    init       app:_init_modules:494 Initializing command history...
18:45:10.216 DEBUG    init       app:_init_modules:497 Initializing websettings...
18:45:10.216 DEBUG    init       webenginesettings:init:477 Initializing qute://* handler...
18:45:10.217 DEBUG    init       webenginesettings:init:481 Initializing request interceptor...
18:45:10.217 DEBUG    init       webenginesettings:init:486 Initializing QtWebEngine downloads...
18:45:10.219 CRITICAL qt         Unknown module:none:0 Could not find QtWebEngineProcess
ERROR
___________ ERROR at setup of test_yank_selection_without_selection ____________

qapp = <PyQt5.QtWidgets.QApplication object at 0x7f7a7589de50>
server = <end2end.fixtures.webserver.WebserverProcess object at 0x7f7a7589dee0>
request = <SubRequest 'quteproc_process' for <Function test_yank_selection_without_selection>>

    @pytest.fixture(scope='module')
    def quteproc_process(qapp, server, request):
        """Fixture for qutebrowser process which is started once per file."""
        # Passing request so it has an initial config
        proc = QuteProc(request)
>       proc.start()

tests/end2end/fixtures/quteprocess.py:1018: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
tests/end2end/fixtures/quteprocess.py:703: in start
    super().start(*args, **kwargs)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <end2end.fixtures.quteprocess.QuteProc object at 0x7f7a64169820>
args = None

    def start(self, args=None, *, env=None):
        """Start the process and wait until it started."""
        self._start(args, env=env)
        self._started = True
        verbose = self.request.config.getoption('--verbose')

        timeout = 60 if testutils.ON_CI else 20
        for _ in range(timeout):
            with self._wait_signal(self.ready, timeout=1000,
                                   raising=False) as blocker:
                pass

            if not self.is_running():
                if self.exit_expected:
                    return
                # _start ensures it actually started, but it might quit shortly
                # afterwards
>               raise ProcessExited('\n' + _render_log(self.captured_log,
                                                       verbose=verbose))
E               end2end.fixtures.testprocess.ProcessExited: 

Log: qutebrowser_20210324_1844.log

The-Compiler commented 3 years ago

Yep, --qute-bdd-webengine is recommended - the only reason it's not the default is that tests with QtWebKit (if available) are a bit faster.

It now seems to fail with "Could not find QtWebEngineProcess" from Qt. Are you setting QTWEBENGINEPROCESS_PATH=/app/bin/QtWebEngineProcess (like here) when running the tests as well?

tinywrkb commented 3 years ago

Right, that's my fault.
I'm still running the tests and I'm seeing lots of errors but only 9 tests are failing except the locale tests.
I'll try later to see if it's my fault that those tests are failing.

Here's the full log: qutebrowser_20210324_2115.log.tar.gz

The-Compiler commented 3 years ago

I think there are some more failing:

397 failed, 8556 passed, 350 skipped, 63 xfailed, 3 errors

So something like 42 failing/erroring tests excluding the locale ones. We're getting there! :sweat_smile:

Could you show a log without the -v -s --no-qt-log arguments? Those are only useful when debugging aborts/segfaults - now that those apparently aren't happening anymore, leaving them off should make everything a bit more digestable.

I haven't looked at most in detail yet, but I think https://github.com/qutebrowser/qutebrowser/commit/9303e6a85489775f63b5d15b7bbd3173f74764d9 will fix some more.

tinywrkb commented 3 years ago

Updated build log with https://github.com/qutebrowser/qutebrowser/commit/9303e6a85489775f63b5d15b7bbd3173f74764d9 pulled in.

qutebrowser_20210324_2236.log

I haven't really looked at it but one thing I did notice is that one of the adblock tests is trying to fetch easylist so maybe it can use a pre-downloaded file that will be fetched with the sources by flatpak-builder.

The-Compiler commented 3 years ago

I was busy with something else over the last couple of days, looking into this now.

I got the tests to run locally with flatpak-builder --force-clean --ccache builddir org.qutebrowser.qutebrowser.yml, though I do seem have a different situation with network connections during the tests? It can't connect to the internet, but the localhost thing works fine. It is in /etc/hosts for the Flatpak image (at least for me), so this should really work fine?

Either way, I think a couple of those failures are caused by your patch replacing localhost with 127.0.0.1 across the codebase. If you tell me how to run the tests without network (ideally without having to disable networking on my host machine), I can look into those in more details. But maybe that's just something on your end?

As for the other tests, I'm looking into them now to see what I can fix before a v2.1.1 release (so that you can hopefully drop some patches!). Might still take me a bit though.

I haven't really looked at it but one thing I did notice is that one of the adblock tests is trying to fetch easylist so maybe it can use a pre-downloaded file that will be fetched with the sources by flatpak-builder.

That seems like a bug FWIW. We do have a copy of easylist in the repo for the tests, and I want the testsuite to run without any internet access. I'll take a look.

tinywrkb commented 3 years ago

If you tell me how to run the tests without network

Unless specifically enabled like the following, flatpak-builder (wrapping flatpak build calls) is running the build process in a sandbox with a different network namespace than the host which is what I mean by no networking.
So you don't need to do anything.

# enable network access during the build process 
build-options:
  build-args:
    - --share=network

You can diff org.qutebrowser.qutebrowser.yml from my tests branch against the one from master to see how to get flatpak-builder to run the tests when building the package.
I'll push these later to master as comments.

+    build-options:
+      test-args:
+        - --env=QTWEBENGINEPROCESS_PATH=/app/bin/QtWebEngineProcess
     build-commands:
       - make --file misc/Makefile install PREFIX=/app
+    run-tests: true
+    test-commands:
+      - env PYTHONPATH=. pytest --qute-bdd-webengine
     sources:
-      - type: archive
-        url: https://github.com/qutebrowser/qutebrowser/releases/download/v2.1.0/qutebrowser-2.1.0.tar.gz
-        sha256: 1ddd373a4f31f16ba809870779918a8920b13dcb936f2d41ff4b27cfd4cae63b
+      - type: git
+        url: https://github.com/qutebrowser/qutebrowser.git
+        tag: v2.1.0
+        commit: 301f65debb2accaf5ec3b7c97e44fc1855758830
The-Compiler commented 3 years ago

I'm running flatpak-builder --force-clean --ccache builddir org.qutebrowser.qutebrowser.yml on the tests branch but with 0006-tests-avoid-loopback-name-resolution.patch commented out, but I don't get the socket.bind issue from https://github.com/flathub/org.qutebrowser.qutebrowser/issues/18#issuecomment-802734451 at all.

Even with a different networking namespace (and no internet access, I can see ~2 tests failing due to that), localhost should be resolvable, and it is for me.

tinywrkb commented 3 years ago

localhost should be resolvable, and it is for me.

This is what I'm seeing on my system.

$ flatpak run --command=curl org.kde.Sdk//5.15 localhost
curl: (6) Could not resolve host: localhost

$ flatpak run --command=curl --share=network  org.kde.Sdk//5.15 localhost
curl: (7) Failed to connect to localhost port 80: Connection refused
The-Compiler commented 3 years ago

I get

$ flatpak run --command=curl org.kde.Sdk//5.15 localhost
curl: (7) Failed to connect to localhost port 80: Connection refused

as I would expect, because it's in /etc/hosts (apparently the same as on my local machine):

$ flatpak run --command=cat org.kde.Sdk//5.15 /etc/hosts 
# Static table lookup for hostnames.
# See hosts(5) for details.
127.0.0.1 localhost
::1 localhost
127.0.1.1 aragog.localdomain aragog
tinywrkb commented 3 years ago

So it's host-dependent and it doesn't seem to react well to changes. It seems to be somehow cached between sessions.
I guess it's my fault as my host is semi-stateless and if I really need to make temporary changes to read-only system settings then I just bind-mount over the specific file.
I will send a PR to check if the CI is being affected by this.

The-Compiler commented 3 years ago

FWIW I've made some progress with the tests, and with

diff --git i/org.qutebrowser.qutebrowser.yml w/org.qutebrowser.qutebrowser.yml
index ecd53db..f8ac2d2 100644
--- i/org.qutebrowser.qutebrowser.yml
+++ w/org.qutebrowser.qutebrowser.yml
@@ -43,22 +43,9 @@ modules:
     sources:
       - type: git
         url: https://github.com/qutebrowser/qutebrowser.git
-        tag: v2.1.0
-        commit: 301f65debb2accaf5ec3b7c97e44fc1855758830
+        branch: dev
       - type: patch
         path: 0001-Remove-manpage-support.patch
-      - type: patch
-        path: 0002-Fix-version-parsing-with-Flatpak.patch
-      - type: patch
-        path: 0003-Use-correct-runtime-path-for-Flatpak.patch
-      - type: patch
-        path: 0004-Add-a-test-for-flatpak-runtime-dir.patch
-      - type: patch
-        path: 0005-standarddir-Fix-custom-basedirs-with-flatpak.patch
-      - type: patch
-        path: 0006-tests-avoid-loopback-name-resolution.patch
-      - type: patch
-        path: 0007-tests-Don-t-download-TLD-list.patch
     modules:
       - python3-colorama.json
       - name: python3-importlib-resources
@@ -133,6 +120,7 @@ modules:
               - --concatenate
               - --confirm-license
               - --enable=QtCore
+              - --enable=QtDBus
               - --enable=QtGui
               - --enable=QtNetwork
               - --enable=QtOpenGL

I only get two failing tests remaining. Will look at those tomorrow (hopefully), and then backport everything to v2.1.x and probably release a v2.1.1 soon.

tinywrkb commented 3 years ago

The CI is building the tests in #23.
Here the x86_64 build log: https://flathub.org/builds/#/builders/19/builds/5108

The-Compiler commented 3 years ago

The current v2.1.x branch fixes those tests as well. I'm planning to release v2.1.1 later today or tomorrow.

As for the aarch64 tests, I'll investigate those once I have access to some hardware to run qutebrowser on (ran into GLX issues when trying on Amazon AWS): https://github.com/qutebrowser/qutebrowser/issues/6347

tinywrkb commented 3 years ago

I'm running another CI build instance with the tests CI against the v2.1.x branch.

It's now easier to enable the tests, this is the only change needed.
I switched to a python3-tests-dependencies.json that was generated from tests-requirements.txt. If nothing is missing from it and it's working correctly then less maintenance work is needed.

The python3-adblock and maturin modules are now being cached correctly so local rebuilds are much faster.

https://flathub.org/builds/#/builders/32/builds/43556

tinywrkb commented 3 years ago

x86_64

========== 8962 passed, 352 skipped, 63 xfailed in 1075.42s (0:17:55) ==========

tinywrkb commented 3 years ago

aarch64

= 5 failed, 8957 passed, 352 skipped, 63 xfailed, 1 rerun in 1589.55s (0:26:29) =

The-Compiler commented 3 years ago

Great, looks as expected now! I won't be able to fix the aarch64 test at the moment, so you might need to skip those (I can suggest a patch for that tomorrow).

The-Compiler commented 3 years ago

FYI, v2.1.1 is released now!

tinywrkb commented 2 years ago

I'm not convinced that we should enable tests for flathubbot builds.
OBS Github repository has Flatpak integration for its CI, so maybe follow it as an example.

Closing, as the tests are running correctly for a while now, and there's nothing to do here.