Repository navigation
test(validation,python): name the tokens on each side when a registry pin fails - #670
Conversation
… pin fails
The three tests that pin SQLGLOT_DIALECTS / NON_SQL_DIALECTS, OssieDialect
and OssieVendor to the tables in extensions/ stop with "do not list the
same dialects" or "... vendors", and the reader has to work out from
pytest's set diff which side the token is missing from. A registration
touches a table row and a line of code, so the message now names the
file to edit:
validate.py and extensions/dialects.md do not list the same dialects:
in extensions/dialects.md but not in validate.py: BAR; in validate.py
but not in extensions/dialects.md: FOO
One helper builds the whole message from the registry kind, so each
assertion is a single line; it is repeated in the two test files the way
_registered_tokens already is.
Suggested as a follow-up in the review of the registry pin tests.
christianeu-db
left a comment
christianeu-db
left a comment
There was a problem hiding this comment.
Thanks for picking this up. I forced mismatches on both sides after merging main, and the messages look good. Both suites pass.
One suggestion for validate.py: the old message output SQLGLOT_DIALECTS / NON_SQL_DIALECTS, while the new one only says "not in validate.py". Contributors still need to know which table to update. Could we retain that detail?
for example,
assert known == registered, _registry_mismatch(
"dialects", known, registered,
"validate.py (SQLGLOT_DIALECTS or NON_SQL_DIALECTS)"
)
Two minor description corrections:
The SDK message names the enum, not the file. Using "OssieDialect (python/src/ossie/models.py)" would make the description accurate.
test_validate.py uses _registered_dialects, not _registered_tokens.
Non-blocking: code labels listed but is separated from it by two parameters. Ordering the arguments as (kind, code, in_code, in_registry) would reduce the risk of swapping sets and getting a plausible but incorrect message. A one-sided mismatch test would also cover the most likely failure case.
… side first The dialect pin message now says which table in validate.py to edit, SQLGLOT_DIALECTS or NON_SQL_DIALECTS, as the old message did. The helper takes the code side before the registry side, next to the label that names it, and its test covers a one-sided mismatch.
|
Done in one commit on top. The dialect message names the tables again, the helper takes the code side before the registry side, and its test covers a one-sided mismatch: Description fixed on both points. Both workflows pass locally on current main. |
Follow-up to the #612 review. The three tests that pin SQLGLOT_DIALECTS / NON_SQL_DIALECTS, OssieDialect and OssieVendor to the tables in extensions/ stopped with "do not list the same dialects", and the reader had to work out from pytest's set diff which side the token was missing from. A registration touches a table row and a line of code, so the message now names what to edit: the two tables in validate.py, or the enum in python/src/ossie/models.py. With a token missing on each side:
One helper builds the whole message from the registry kind and the code label, so each assertion is a single line. It is the same nine lines in both test files, the way _registered_dialects and _registered_tokens already are, with a unit test each. Test files only; no change to validate.py or the SDK.