Backfill integration tests for post-2019 changes and fix seven bugs they found - #103
Open
MrIron-no wants to merge 9 commits into
Open
Backfill integration tests for post-2019 changes and fix seven bugs they found#103MrIron-no wants to merge 9 commits into
MrIron-no wants to merge 9 commits into
Conversation
Add pytest suites for behaviour changes made on the release branch that
had no coverage in the harness (each module docstring names the commits):
chanmodes/ channel modes +u (part/quit message suppression edge cases)
and +M (moderate unauthenticated users)
cap/ capability list, extended-join on every JOIN path,
echo-message (plus edge cases)
relay/ NOTICE nick@server rules, JOIN target limits (JOIN_TARGET),
CPRIVMSG idle reset
commands/ WHOWAS <nick> 0, WHOX %l field, PART, INFO, CONNECT 0,
PRIVS defaults, remote STATS parameter forwarding
features/ Boolean feature spellings (0/1), HIS_REMOTE gating,
OPLEVELS/ZANNELS defaults, removed MAXIMUM_LINKS
s2s/ parser robustness (bad numerics, END_OF_BURST token),
GLINE reason/lifetime updates from servers
username/ ident lookups only with a username mask, WebIRC username
trust, STRICT_USERNAME digit-group and case rules
iauth/ STATS iauth / iauthconf, asynchronous "? stats2",
control-character handling in IAuth input
config/ Include directive and lexer via "ircd -k"
Harness changes: make_client(caps=...), an "oper" fixture, docker_exec /
docker_cp_text helpers, split P10 handshake methods on P10Server (as on
the feature branches), and a shared common.py (join/drain/whois and
set_feature, which waits for the RPL_FEATURE reply instead of sleeping
because ircu defers a client's commands once its flood penalty builds
up). leaf2 now runs a non-forcing IAuth stub (iauth-test.pl) with ident
lookups and a WebIRC port; the hub sets AUTH_TIMEOUT=3 and
MAXCHANNELSPERUSER=40 and gives notulined.test.net a port.
Six strict xfails document bugs found while writing the tests: the
lexer has no FROM token, a missing or empty Include file breaks parsing
(hang / syntax error), a self-including file aborts ircd, mo_info still
uses a stale line offset, and CPRIVMSG/CNOTICE are not echoed.
The grammar accepts "Include <types> from "file";" to restrict which block types an included file may contain, but the hand-coded lexer (844238f) never emitted the FROM token, so that form was always a syntax error. Add the keyword to the token table. The restricted-include tests in tests/config/ now pass; a new test checks that a forbidden block type is reported.
lexer_open() keeps a lex_file whose fd is -1 when an Include target cannot be opened, and yylex() returned TOKERR for it on every call. The parser's error recovery never found a token it could resync on, so "ircd -k" (and a REHASH) spun forever on a missing include file. Treat an unopenable file like an empty one: pop it and return TEOF (or end of input if it was the main file) so parsing ends normally with the "error opening file" diagnostic already reported.
An included file that includes itself (directly or through another
file) recursed until the parser stack overflowed ("memory exhausted"),
after which deinit_lexer()'s assert(!yy_in) aborted the daemon -- for a
REHASH that means the running server dies.
lexer_include() now walks the input stack: a file that is already being
read is reported as "recursive include", and nesting deeper than
MAX_INCLUDE_DEPTH (16) as "include nesting too deep". In both cases an
input that yields no tokens is pushed so the Include block still ends
with TEOF and parsing continues. deinit_lexer() unwinds whatever is
left on the stack instead of asserting.
The Include rule required at least one block in the included file
("blocks TEOF"), so a file containing only comments -- or one that
could not be opened -- produced a spurious "syntax error" after the
real diagnostic. Accept an empty include body.
m_info() and ms_info() were changed to end the public text at the "Sources:" line instead of a hard-coded offset, but mo_info() still skipped text[218] entries. Since the number of hashed source files has grown, an operator asking "INFO <server>" silently lost the first part of the hash list (IPcheck.c ... channel.c never appeared) and an operator without a server argument got no public text at all. Use the same marker logic in mo_info(): everybody gets the public text, and operators who name a server get the complete hash list after it.
whisper() delivered the message to the target but, unlike the PRIVMSG/NOTICE relay paths (and WALLCHOPS/WALLVOICES), never sent the echo-message copy back to the sender. Add it after a successful delivery so clients with the capability see their own CPRIVMSG and CNOTICE traffic like every other message.
Since 5ffe0a1, JOIN_TARGET=FALSE (the default) lets a client join channels beyond its free-target budget and defers the target charge to the first message on the channel. check_target_join() still called check_target_limit(), which had already sent ERR_TARGETTOOFAST and added two seconds to cli_nexttarget before returning: every allowed join past the budget produced a spurious "Target change too fast" reply immediately followed by the successful JOIN, and a 25-channel burst left the client with 30 seconds of extra target penalty. Split check_target_limit() into a static core with a "report" flag and add check_target_limit_quiet(), which returns the verdict without sending the numeric or applying the penalty; use it when JOIN_TARGET is off. The JOIN_TARGET=TRUE path is unchanged. Document JOIN_TARGET in readme.features, which never listed it.
The fixture always registered as "testop". ircu defers a client's commands once its flood penalty builds up, so the previous test's QUIT could still be pending when the next test registered, which failed with ERR_NICKNAMEINUSE (seen once in a full-suite run).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Backfills integration tests for the behaviour changes made directly on the release branch since 2019 that had no coverage in the pytest harness, and fixes seven bugs those tests uncovered. Each fix is its own commit and flips the test that documented it (strict
xfail) into a passing test.Tests (
337acf9,50a05e5)174 new test functions (≈200 items), each module docstring naming the commits it pins down:
tests/chanmodes/&local, empty comment, cross-server quit rewrite, CLEARMODE), +M moderation of unauthenticated userstests/cap/(4 new modules)tests/relay/NOTICE nick@serverrules, JOIN target limits (JOIN_TARGET), CPRIVMSG idle reset and error numericstests/commands/WHOWAS <nick> 0, WHOX%l, PART, INFO,CONNECT <server> 0, PRIVS defaults, remote STATS parameter forwardingtests/features/0/1/YES/ON…),HIS_REMOTEgating, defaults, removedMAXIMUM_LINKS, bad values, privilegestests/s2s/END_OF_BURSTfull token, junk lines), GLINE reason/lifetime updates from serverstests/username/tests/iauth/STATS iauth/STATS iauthconf, asynchronous? stats2, concurrent requests, control-character handlingtests/config/Includedirective and the configuration lexer viaircd -kinside the hub containerHarness additions:
make_client(caps=...), anoperfixture,docker_exec/docker_cp_text, split P10 handshake methods onP10Server(as on the feature branches),common.py(join/drain/whois/set_feature—SETis waited for rather than slept on, because ircu defers a client's commands once its flood penalty builds up). leaf2 now runs a non-forcing IAuth stub (iauth-test.pl, policyARUS) with ident lookups and a WebIRC port; the hub setsAUTH_TIMEOUT=3,MAXCHANNELSPERUSER=40and givesnotulined.test.neta port. README updated.Fixes
209c1feFROM, soInclude <types> from "file";was always a syntax error.8575b05yylex()returnTOKERRforever;ircd -k/ REHASH hung. Unopenable inputs are now popped like an empty file.9b2c888deinit_lexer()'sassert(!yy_in)aborted the daemon. Recursive includes and nesting deeper than 16 are reported instead;deinit_lexer()unwinds.5ca7fe5syntax error.cd73030mo_info()still used the staletext[218]offset afterm_info/ms_infomoved to the"Sources:"marker; opers lost the first source hashes as the file count grew.d62e82bwhisper()never sent the echo-message copy for CPRIVMSG/CNOTICE.045198eJOIN_TARGET=FALSEan allowed join past the target budget still gotERR_TARGETTOOFAST(immediately followed by the successfulJOIN) and 2 s of penalty each. Addedcheck_target_limit_quiet(); documentedJOIN_TARGETinreadme.features.Verification
Full repository suite on this branch (all topologies): 843 passed, 1 error — the error was the
operfixture's fixed nick colliding with a still-pendingQUIT, fixed in50a05e5and re-verified. New suites alone: 202 passed, 0 xfailed.Known but left alone (design, not a bug): recent-target tracking hashes the channel pointer and is inherited per IP, so a freshly freed channel's memory reused for a new channel counts as an already-known target.