ci: run the unsafe websocket cases and the unprivileged configuration tests

[andypost]

Two changes that turn on tests which have never run in CI.

The three Autobahn websocket cases are gated on --unsafe, and no leg had ever passed that flag.
Two unconditional @pytest.mark.skip('not yet, unsafe') marks and three in-body
if not is_unsafe: pytest.skip() gates collapse into one @pytest.mark.skipif(not option.unsafe),
evaluated at collection because option.unsafe is set in pytest_configure.

test_configuration.py also carried two cases marked unsafe, but they are a different thing: they
assert that the controller rejects an empty listener object, which it does not, because no member of
nxt_conf_vldt_listener_members is required. Those are known-broken, not slow, so they are
non-strict xfail rather than flag-gated. That keeps --unsafe meaning "slow" and nothing else, so
a plain pytest --unsafe test/ is no longer guaranteed to fail, and the day the controller starts
rejecting them an XPASS says so instead of silence. Both already XPASS in one leg, which is exactly
the signal skip never gave.

With that separation the flag needs no per-leg logic, so every leg passes it and picks up whichever
Autobahn case its own testpath contains.

What turning the flag on found

Node passes on all four versions. The other two do not, and the failures are real module bugs, not
flakes or timeouts. Every leg finished in about four minutes against a thirty minute budget, so the
'long run' reason had been hiding defects rather than minutes for years.

leg	result	cause
node 20/22/24/26	pass
python 3.12/3.13/3.14	UnicodeDecodeError	#429, the ASGI module enforces a private 1 MiB frame cap and ignores max_frame_size
java 17/21/25/26	AssertionError: fin	#430, the java module fragments any text message above 8 KiB and ignores max_frame_size

The asgi and java cases are therefore marked non-strict xfail against those two issues, keeping
their skipif so they still only run under the flag. The coverage lands green, the bugs stay
visible and attributed, and fixing either issue turns its case into an XPASS. Dropping --unsafe
again would put both defects back into silence, which is the one outcome worth avoiding.

The unprivileged step

test_configuration.py::test_unprivileged_user_error requires a unitd that is not root, the reverse
of every sudo'd leg, so it had never run either. The non-sudo step now runs that file alongside the
capget fallback tests. The file's other cases need no privilege: its only require() is the
unprivileged one, and it talks solely to the control socket.

The dead is_unsafe fixture goes with the gates that used it, and option.unsafe gains a default so
the module-level marks do not make those files unimportable outside a pytest run.

Rebased onto master after #418, keeping its set -o pipefail and tee and adding --unsafe to both
invocations. No longer waits on anything.

🤖 Generated with Claude Code

[andypost]

@claude review

[claude]

Claude finished @andypost's task in 3m 58s —— View job

---

Review: ci: run the unsafe websocket cases and the unprivileged configuration tests

• Read the PR diff against origin/master
• Trace --unsafe plumbing (pytest_addoption → pytest_configure → option.unsafe)
• Check the per-leg testpath logic to validate the "node only" justification
• Check test_configuration.py for privilege assumptions
• Post review

The mechanics are right. option.unsafe is set in pytest_configure (test/conftest.py:106), which runs before test modules are imported, so the collection-time skipif reads a populated value — the reasoning in the description checks out. The node testpath (test/test_node* test/test_app_lifecycle.py) genuinely excludes test_configuration.py, and test_configuration.py's only require() is {'privileged_user': False} (test/test_configuration.py:669), so the unprivileged step's premise holds.

Findings below, most important first.

---

1. The test_configuration.py hunk makes pytest --unsafe test/ permanently red — and it enables nothing

test/test_configuration.py:223,357 turn two @pytest.mark.skip('not yet, unsafe') into @pytest.mark.skipif(not option.unsafe, ...). Those two cases assert behaviour the controller does not implement: under --unsafe they fail, they don't run.

No leg runs that file with --unsafe, so this hunk turns on zero CI coverage. What it does buy is a lasting constraint: it is the sole reason the workflow needs a per-leg $unsafe variable and the six-line comment explaining it, and it means a plain pytest --unsafe test/ — the obvious thing a developer types — is now guaranteed to fail on two cases that were designed to be skipped.

xfail is the mark for "known broken", and it keeps --unsafe meaning "slow, not broken":

@pytest.mark.xfail(reason='not yet: controller accepts a type-only application')
def test_applications_type_only():

Non-strict xfail also gives you the signal skip never did — the day the controller starts rejecting these, you get an XPASS instead of silence. Then --unsafe can be passed unconditionally on every leg and the workflow keeps its single pytest-3 line, which also makes the #418 reconciliation you flagged a non-event.

Leaving the marks as skip is the other acceptable answer; either way this hunk should not be what dictates the workflow's shape.

Fix this →

2. Two of the three Autobahn cases the PR is named after still never run

The workflow comment justifies node with "no other --unsafe-gated test is in its testpath". That is equally true of java: test/test_java* contains only @pytest.mark.skip('not yet') marks (test_java_websockets.py:174,209,243,496), which stay skipped regardless of the flag, plus the Autobahn case itself. So java could carry --unsafe today, under the comment's own rule.

The real constraint is the python legs, where the sharded testpath can include test_configuration.py. Worth either giving java the flag too, or narrowing the comment to say the choice is "one leg to start with", so the next reader doesn't take it as a technical limit. As it stands the PR title promises three cases and delivers one.

3. Runtime headroom on the node leg

The Autobahn case is --unsafe-gated for "long run", not just for flakiness: check_payload(op_text, 2**20, chopsize=64) is 16384 socket writes, and there are twelve check_payload calls plus 1/4/16 MiB check_message calls (test/test_node_websockets.py:1330-1356). The test job is timeout-minutes: 30 (line 209) for all legs. Since the whole point of the PR is that this has never run, please read the measured node-leg duration off this PR's own CI run before merge — and if it eats real headroom, either bump the timeout for node or give the case its own step so a timeout is attributable rather than looking like a hang in an unrelated test.

4. is_unsafe fixture is now dead

test/conftest.py:977 has zero consumers after this PR — it was the only mechanism the three removed in-body gates used. Collapsing the gating is this PR's stated job; the fixture should go with them.

Fix this →

5. Minor: option.unsafe has no default

Options.__getattr__ raises AttributeError for anything not in _options (test/unit/option.py:22), and unsafe is only ever injected by pytest_configure. Reading it at module import is safe inside a pytest run, but the new skipif marks make four test modules unimportable outside one. Adding 'unsafe': False to the _options defaults costs a line and removes the ordering coupling entirely.

6. Note on the unprivileged step

test_configuration.py now runs on all three python legs (module == 'python') — ~60 cases, three times, for the one that needs non-root. That matches what the capget step already does, so it is consistent rather than new, but the shard-0 gate the workflow already uses for test_app_lifecycle.py would be the cheaper shape. Two cases there are worth watching on the first green run, since this step is !cancelled() and therefore always reports: test_json_application_many2 (100 applications, fd-limit sensitive — the file comments on this itself) and test_listeners_port_release, both running as the runner user after a sudo'd pytest has already been through the same tree.

---

Nothing here blocks the approach — the --unsafe plumbing and the unprivileged step are both correct as written. Item 1 is the one I'd want changed before merge, because it is the piece that constrains the workflow rather than the other way round.
· branch ci/m1-unsafe-and-unprivileged

[andypost]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fddba34c03

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Keep the unfinished listener test unconditionally skipped

When a developer runs pytest --unsafe test or targets test_configuration.py, this marker now enables test_listeners_empty, which still asserts behavior explicitly labeled “not yet”; the current listener validation accepts an empty object because none of nxt_conf_vldt_listener_members is required. The workflow avoids the failure only by passing --unsafe to the node-only test path, so the general unsafe suite is now guaranteed to fail. Leave this test unconditionally skipped until empty listeners are rejected.

Useful? React with 👍 / 👎.

[andypost]

The seven red legs are not defects in this PR. The Autobahn payload-size cases it enables have never run in CI, and they found two real module bugs, now filed:

• websocket: the ASGI module rejects a message over a private 1 MiB cap, ignoring max_frame_size #429 — ASGI: nxt_py_asgi_ws_max_frame_size is a private 1 MiB static (src/python/nxt_python_asgi_websocket.c:97, never written) enforced at :522, so the configured 32 MiB max_frame_size is a no-op and a 4 MiB message closes with 1009. Accounts for python-3.12/3.13/3.14.
• websocket: the java module fragments every text message above 8 KiB, ignoring max_frame_size #430 — java: Constants.DEFAULT_BUFFER_SIZE is 8 KiB and WsRemoteEndpointImplBase.java:254 sets fin per encode-buffer fill, so a 64 KiB echo returns as 8 frames and test_java_websockets.py:51 fails on fin. Accounts for java-17/21/25/26.

Suggest xfailing test_asgi_websockets_9_1_1__9_6_6 and test_java_websockets_9_1_1__9_6_6 against those two issues so the coverage lands green, rather than dropping --unsafe again — that flag is what made these visible after years of silence.

Note this branch is also CONFLICTING against master and needs a rebase.