Skip to content

test: make diagnostic helper name match behavior - #1054

Merged
CppCXY merged 1 commit into
EmmyLuaLs:mainfrom
lewis6991:test/invert-check-code-for
Apr 29, 2026
Merged

CppCXY merged 1 commit into
EmmyLuaLs:mainfrom
lewis6991:test/invert-check-code-for

Conversation

@lewis6991

@lewis6991 lewis6991 commented Apr 29, 2026 •

Copy link
Copy Markdown
Collaborator

Rename check_code_for to has_no_diagnostic so assertions describe the
existing behavior directly.

Assisted-by: Codex

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request inverts the boolean logic of the check_code_for function within the test library, resulting in a widespread update to assertions across the test suite to ensure tests continue to validate the presence or absence of diagnostics correctly. I have no feedback to provide.

@CppCXY

CppCXY commented Apr 29, 2026

Copy link
Copy Markdown
Member

If the logic is to be completely reversed, that usually means the current name is not appropriate. I would suggest renaming it rather than reversing the judgment logic.

@lewis6991

Copy link
Copy Markdown
Collaborator Author

A better name with the old logic would be check_no_code_for, but that just embeds the ! into the name. I do think keeping the name and reversing the result is better here. check_code_for means "run this code and check it raises this diagnostic".

@CppCXY

CppCXY commented Apr 29, 2026

Copy link
Copy Markdown
Member

A better name with the old logic would be , but that just embeds the into the name. I do think keeping the name and reversing the result is better here. means "run this code and check it raises this diagnostic".check_no_code_for``!``check_code_for

The problem with reversing the logic is that it would result in unmerged code, which would naturally be reversed in terms of logic precisely because of this PR, and cannot be easily fixed by simple text replacement.

Rename check_code_for to has_no_diagnostic so assertions describe the
existing behavior directly.

Assisted-by: Codex
@lewis6991
lewis6991 force-pushed the test/invert-check-code-for branch from fd63feb to e866608 Compare April 29, 2026 11:43
@lewis6991 lewis6991 changed the title test: make check_code_for mean diagnostic found test: make diagnostic helper name match behavior Apr 29, 2026
@CppCXY
CppCXY merged commit 4675b36 into EmmyLuaLs:main Apr 29, 2026
16 checks passed
@lewis6991
lewis6991 deleted the test/invert-check-code-for branch April 29, 2026 12:35
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