test(dns): expand is_hostname coverage for length, hyphen, dot, and A-label branches#2683
Open
vtushar06 wants to merge 2 commits into
Open
test(dns): expand is_hostname coverage for length, hyphen, dot, and A-label branches#2683vtushar06 wants to merge 2 commits into
vtushar06 wants to merge 2 commits into
Conversation
…-label branches Adds 94 tests for is_hostname. Core already passes all of them, so this is coverage, not a fix. What is new: - the total-length cap reached through many short labels, not just four 63-byte ones, and the 63/64 boundary in both a middle and a last label - hyphen placement: leading, trailing, lone, consecutive, and a 63-byte hyphen-heavy label - dot structure: empty labels, leading and trailing dots, repeated dots - the xn-- A-label branch, including the case-insensitive prefix - line terminators, whitespace, and embedded NUL - non-ASCII UTF-8 in a label - three loops that sweep all 256 byte values in leading, interior and trailing position How I checked the expected values: every input was run through the real is_hostname before being written down, and the file compiles and runs clean (277 passed, 0 failed for the dns target). The tests assert is_hostname only, rather than following the file's existing habit of asserting is_idn_hostname alongside it. A few of these inputs are ones where the two predicates currently disagree, so pairing them here would have baked that in. I will raise that separately. RFC references: - RFC 1123 section 2.1 (host name syntax, first-character relaxation) - RFC 952 (the <name> grammar and its character repertoire) - RFC 1035 sections 2.3.1 and 3.1 (letter/digit sets, length limits) Signed-off-by: Tushar Verma <tusharmyself06@gmail.com>
🤖 Augment PR SummarySummary: This PR expands test coverage for Changes:
Technical Notes: The new tests are structured to hit previously-unreached branches in 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
jviotti
reviewed
Jul 26, 2026
jviotti
reviewed
Jul 26, 2026
jviotti asked for two changes: - drop the multi-line section-banner comments and go back to the file's own convention, one concise RFC-citation line directly above each test (or a tight 2-3 test cluster where they are genuinely the same rule, matching how the existing MIDDLE DOT/GERESH tests already do this) - replace the three loop-based byte-alphabet sweeps with literal sequential EXPECT_TRUE/FALSE calls, split into groups rather than one 256-assertion test. I grouped by ASCII structure (controls, digits, uppercase, lowercase, punctuation, non-ASCII in four quarters), 12 groups per position, 36 tests total, still covering all 256 byte values at each of the three positions Generated the literal expansion from the real is_hostname output so nothing here is hand-guessed, same as the rest of the file. That generation caught a bug in itself: a byte value of 0 written as a plain concatenated string literal truncates on the implicit const char* -> string_view conversion, so a candidate like "a" "\x00" "b" only reaches is_hostname as "a". Fixed by using an explicit-length std::string_view for the three NUL cases, matching how the file's existing embedded-NUL tests already do it. 310 passed, 0 failed on the dns target. clang-format is a stable fixed point. Mutation-checked by flipping three expectations and confirming exactly three failures. Signed-off-by: Tushar Verma <tusharmyself06@gmail.com>
Contributor
Author
|
@jviotti I have addressed both - dropped the section banners for one line per test, and expanded the byte sweep into literal calls grouped by ASCII range. |
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
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.
I added 94 tests for
is_hostname. Core already passes all of them, so this is coverage, not a fix.I went through
src/core/dns/hostname.ccbranch by branch and wrote tests for the ones the existing 66 did not reach.What is new
-, consecutive hyphens, and a 63-byte label that is almost all hyphens.xn--A-label branch, including the case-insensitive prefix match (Xn--,xN--,XN--).std::string_view(..., N)- written as a plain C string literal they would truncate at the NUL and silently test a different input.How I checked the expected values
Every input was run through the real
is_hostnamebefore I wrote the expectation down, so none of these are hand-guessed. The file compiles clean and the dns target reports 277 passed, 0 failed.clang-formatis idempotent on it.One deliberate inconsistency
The existing tests in this file assert
is_idn_hostnamealongsideis_hostname. Mine assertis_hostnameonly. A few of these inputs are ones where the two predicates currently disagree, so pairing them here would have written that disagreement in as expected behaviour. I would rather raise it on its own, which I will do separately.RFC references
<name>grammar and its character repertoire