Skip to content

internal: correct two stale comments in nextest config and parser tests - #6187

Open
prql-bot wants to merge 3 commits into
mainfrom
internal/stale-comments
Open

internal: correct two stale comments in nextest config and parser tests#6187
prql-bot wants to merge 3 commits into
mainfrom
internal/stale-comments

Conversation

@prql-bot

@prql-bot prql-bot commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Comment corrections found by the nightly survey, plus one ordering fix in the same file.

.config/nextest.toml — the queries:: override carried the same "compileall does many passes over the same query" comment as the queries::compileall:: override below it, which reads as a copy-paste. That override covers every query test (compile, fmt, lex, debug_lineage, results, …), not just compileall, so the comment now says what's actually true of all of them — "process" rather than "compile", since lex only lexes and fmt/debug_lineage stop at PL.

.config/nextest.toml — the three queries:: overrides are now ordered specific-first. nextest resolves overrides first-match-wins per setting (per-test overrides) and test(str) is a contains matcher (filterset reference), so the broad test(queries::) filter also matched queries::compileall::* and queries::results::*; being declared first, its 2s slow-timeout won and the 10s values on the two overrides below it were inert. Nothing was killed — no terminate-after is set anywhere — but compileall and results were reported as SLOW after 2s rather than the 10s the config intends. test-group = 'test-dbs' on the results:: override was unaffected either way, since the broad override doesn't configure that setting.

prqlc-parser/src/parser/stmt.rs — "eated" → "eaten" in the doc_comment_inline_module test comment.

Verifying the override ordering

A minimal crate with a test named queries::compileall::slow that sleeps 4s, under nextest 0.9.143, with a 500ms profile default:

Broad-first (the ordering on main):

SLOW [>  2.000s] (───) ntdemo::integration queries::compileall::slow
SLOW [>  4.000s] (───) ntdemo::integration queries::compileall::slow
PASS [   4.004s] (1/1) ntdemo::integration queries::compileall::slow

Specific-first (this PR):

PASS [   4.004s] (1/1) ntdemo::integration queries::compileall::slow

No SLOW lines in the second case — the 10s override applies, as intended.

@prql-bot prql-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Self-review. One wording nit inline, plus one observation about the file this touches.

.config/nextest.toml — nextest resolves overrides first-match-wins per setting: "the first override within profile.my-profile.overrides that matches T and configures S is applied" (per-test overrides), and test(str) defaults to a contains matcher (filterset reference). So the test(queries::) filter also matches queries::compileall::* and queries::results::*, and because it's declared first its 2s slow-timeout wins — the 10s values on the two overrides below it are inert. (test-group = 'test-dbs' on the results:: override still applies, since the first override doesn't configure that setting.) No test is killed by this, as no terminate-after is set anywhere; the effect is just that compileall and results tests get reported as SLOW after 2s rather than 10s. Pre-existing and out of scope for a comment fix — the change would be to move the broad queries:: override below the two specific ones. Happy to open a follow-up if that reading looks right.

Comment thread .config/nextest.toml Outdated
@prql-bot

Copy link
Copy Markdown
Collaborator Author

Verified the override-precedence reading empirically rather than opening a follow-up, and folded the one-hunk fix into this PR — it's the same four lines of the same file as the comment correction, so a stacked PR would only have created a conflict for whoever merged second. A minimal crate with a queries::compileall::slow test that sleeps 4s reports SLOW [> 2.000s] with the broad test(queries::) override declared first, and no SLOW line at all once the specific overrides move above it (output in the PR description).

If you'd rather keep this PR to comments only, say so and I'll pull the reordering out into its own PR.

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.

1 participant