Skip to content

make metadata tests compatible with batch parameter #314

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

Merged
merged 2 commits into from
Jun 25, 2025

Conversation

slingamn
Copy link
Collaborator

We're updating the spec to add a batch parameter to metadata. This makes the tests compatible with both versions of the spec for now; eventually we'll update Ergo and the Unreal module to match the new spec, at which point we can make the batch parameter a hard requirement.

We're updating the spec to add a batch parameter to metadata.
This makes the tests compatible with both versions of the spec for now;
eventually we'll update Ergo and the Unreal module to match the new spec,
at which point we can make the batch parameter a hard requirement.
slingamn added a commit to slingamn/ergo that referenced this pull request Jun 22, 2025
This is progval/irctest#314 but I don't want to
couple the merges
slingamn added a commit to ergochat/ergo that referenced this pull request Jun 22, 2025
* spec update: metadata keys are lowercase

* add batch parameter to metadata batches

* fix: connecting clients receive METADATA, not RPL_KEYVALUE

* spec update: send RPL_METADATASUBS in a metadata-subs batch

* move some helpers

* bump irctest to forked hash

This is progval/irctest#314 but I don't want to
couple the merges

* fix: empty value is valid

* fix: deleting a nonexistent key gets a FAIL
Comment on lines +27 to 31
# TODO: s/ANYOPTSTR/ANYSTR/, as per spec update to require a batch parameter
# indicating the target
self.assertMessageMatch(
first_msg, command="BATCH", params=[StrRe(r"\+.*"), "metadata"]
first_msg, command="BATCH", params=[StrRe(r"\+.*"), "metadata", ANYOPTSTR]
)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make the caller pass the expected target name, and match with OptStrRe(re.escape(target))

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kind of painful because we can't make an Either optional AFAICT. It'll be easier to do after we make the parameter mandatory.

@progval progval merged commit ccdacd9 into progval:master Jun 25, 2025
36 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants