nginx / unit

NGINX Unit - universal web app server - a lightweight and versatile open source server that simplifies the application stack by natively executing application code across eight different programming language runtimes.
https://unit.nginx.org
Apache License 2.0
5.4k stars 331 forks source link

Current git HEAD tests fail if Unit is built without njs: test_routes_match_if #1411

Closed thresheek closed 2 months ago

thresheek commented 2 months ago

When built without njs, the test suite fails:

The snippet is from Fedora 39, but I believe it happens everywhere when built like: ./configure && ./configure python && make && make python

[thresh@ip-10-1-18-178]~/unit2% python3 -m pytest test/test_routing.py -k test_routes_match_if
error: no such command: `component`

    View all installed commands with `cargo --list`
    Find a package to install `component` with `cargo search cargo-component`
================================================================================================================ test session starts ================================================================================================================
platform linux -- Python 3.12.0, pytest-7.3.2, pluggy-1.2.0 -- /usr/bin/python3
cachedir: .pytest_cache
rootdir: /home/thresh/unit2/test
configfile: pytest.ini
collected 114 items / 113 deselected / 1 selected

test/test_routing.py::test_routes_match_if FAILEDPath to unit.log:
/tmp/unit-test-87vbse4g/unit.log

2024/09/06 00:31:20 [warn] 166427#166427 Unit is running unprivileged, then it cannot use arbitrary user and group.
2024/09/06 00:31:20 [info] 166427#166427 unit 1.33.0 started
2024/09/06 00:31:20 [info] 166428#166428 discovery started
2024/09/06 00:31:20 [notice] 166428#166428 module: python 3.12.0 "/home/thresh/unit2/build/lib/unit/modules/python.unit.so"
2024/09/06 00:31:20 [info] 166427#166427 controller started
2024/09/06 00:31:20 [notice] 166427#166427 process 166428 exited with code 0
2024/09/06 00:31:20 [info] 166430#166430 router started

===================================================================================================================== FAILURES ======================================================================================================================
_______________________________________________________________________________________________________________ test_routes_match_if ________________________________________________________________________________________________________________

    def test_routes_match_if():

        def set_if(condition):
            assert 'success' in client.conf(f'"{condition}"', 'routes/0/match/if')

        def try_if(condition, status):
            set_if(condition)
            assert client.get(url=f'/{condition}')['status'] == status

        assert 'success' in client.conf(
            {
                "listeners": {"*:8080": {"pass": "routes"}},
                "routes": [
                    {
                        "match": {"method": "GET"},
                        "action": {"return": 200},
                    }
                ],
                "applications": {},
            }
        ), 'routing configure'

        # const

        try_if('', 404)
        try_if('0', 404)
        try_if('false', 404)
        try_if('undefined', 404)
        try_if('!', 200)
        try_if('!null', 200)
        try_if('1', 200)

        # variable

        set_if('$arg_foo')
        assert client.get(url='/bar?bar')['status'] == 404
        assert client.get(url='/foo_empty?foo')['status'] == 404
        assert client.get(url='/foo?foo=1')['status'] == 200

        set_if('!$arg_foo')
        assert client.get(url='/bar?bar')['status'] == 200
        assert client.get(url='/foo_empty?foo')['status'] == 200
        assert client.get(url='/foo?foo=1')['status'] == 404

        # njs

>       set_if('`${args.foo == \'1\'}`')

test/test_routing.py:2060:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

condition = "`${args.foo == '1'}`"

    def set_if(condition):
>       assert 'success' in client.conf(f'"{condition}"', 'routes/0/match/if')
E       assert 'success' in {'detail': 'Unit is built without support of njs: "--njs" ./configure option is missing. in the "if" value.', 'error': 'Invalid configuration.'}
E        +  where {'detail': 'Unit is built without support of njs: "--njs" ./configure option is missing. in the "if" value.', 'error': 'Invalid configuration.'} = <bound method args_handler.<locals>.args_wrapper of <unit.applications.lang.python.ApplicationPython object at 0x7fd9c6b15b80>>('"`${args.foo == \'1\'}`"', 'routes/0/match/if')
E        +    where <bound method args_handler.<locals>.args_wrapper of <unit.applications.lang.python.ApplicationPython object at 0x7fd9c6b15b80>> = <unit.applications.lang.python.ApplicationPython object at 0x7fd9c6b15b80>.conf

test/test_routing.py:2017: AssertionError
============================================================================================================== short test summary info ==============================================================================================================
FAILED test/test_routing.py::test_routes_match_if - assert 'success' in {'detail': 'Unit is built without support of njs: "--njs" ./configure option is missing. in the "if" value.', 'error': 'Invalid configuration.'}
========================================================================================================= 1 failed, 113 deselected in 0.15s =========================================================================================================
ac000 commented 2 months ago

Hmm, I guess that test should be dependant on njs...

ac000 commented 2 months ago

Something like this...

diff --git ./test/test_routing.py ./test/test_routing.py
index c419779a..ce323277 100644
--- ./test/test_routing.py
+++ ./test/test_routing.py
@@ -2057,6 +2057,9 @@ def test_routes_match_if():

     # njs

+    if not option.available['modules']['njs']:
+        return
+
     set_if('`${args.foo == \'1\'}`')
     assert client.get(url='/foo_1?foo=1')['status'] == 200
     assert client.get(url='/foo_2?foo=2')['status'] == 404
hongzhidao commented 2 months ago

Something like this...

diff --git ./test/test_routing.py ./test/test_routing.py
index c419779a..ce323277 100644
--- ./test/test_routing.py
+++ ./test/test_routing.py
@@ -2057,6 +2057,9 @@ def test_routes_match_if():

     # njs

+    if not option.available['modules']['njs']:
+        return
+
     set_if('`${args.foo == \'1\'}`')
     assert client.get(url='/foo_1?foo=1')['status'] == 200
     assert client.get(url='/foo_2?foo=2')['status'] == 404

Good fixing, thanks.

thresheek commented 2 months ago

Thanks, that patch seems to work for me too, as the tests are now skipped.