Skip to content

[TASK] Use DeclarationList in the tests#1541

Merged
oliverklee merged 3 commits intomainfrom
task/tests/declarationlist
Feb 20, 2026
Merged

[TASK] Use DeclarationList in the tests#1541
oliverklee merged 3 commits intomainfrom
task/tests/declarationlist

Conversation

@JakeQZ
Copy link
Copy Markdown
Collaborator

@JakeQZ JakeQZ commented Feb 20, 2026

The interface was renamed from RuleContainer in #1530.

Also accordingly rename the trait that provides test methods for implementing classes.

@JakeQZ JakeQZ requested a review from oliverklee February 20, 2026 18:15
@JakeQZ JakeQZ self-assigned this Feb 20, 2026
@JakeQZ JakeQZ added testing PRs/issues adding additional tests only, or primarily testing-focused developer-specific Issues that only affect maintainers, contributors, and people submitting PRs refactor For PRs that refactor code without changing functionality labels Feb 20, 2026
The interface was renamed from `RuleContainer` in #1530.

Also accordingly rename the trait that provides test methods for implementing
classes.
@JakeQZ JakeQZ force-pushed the task/tests/declarationlist branch from bd6a3e5 to 6f28853 Compare February 20, 2026 18:19
@JakeQZ
Copy link
Copy Markdown
Collaborator Author

JakeQZ commented Feb 20, 2026

For some reason the CI build can't find the renamed class, but it works fine on my machine.

@JakeQZ JakeQZ marked this pull request as draft February 20, 2026 18:23
Copy link
Copy Markdown
Collaborator

@oliverklee oliverklee left a comment

Choose a reason for hiding this comment

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

The casing of the directory is incorrect (unit instead of Unit). I guess that's hard to fix on Windows. I'll take care of it.

@oliverklee
Copy link
Copy Markdown
Collaborator

While we're renaming the trait anyway, I'd prefer we don't use the Test prefix for it. This prefix should only be used for testcase classes, not for traits. (Also, this will allow us to have a DeclarationListTest in the future if we want it.)

@coveralls
Copy link
Copy Markdown

coveralls commented Feb 20, 2026

Coverage Status

coverage: 72.384%. remained the same
when pulling 1b9c53d on task/tests/declarationlist
into eaa13f3 on main.

@oliverklee
Copy link
Copy Markdown
Collaborator

For some reason the CI build can't find the renamed class, but it works fine on my machine.

… which is a Windows machine that uses a case-insensitive file system. 😄

@JakeQZ
Copy link
Copy Markdown
Collaborator Author

JakeQZ commented Feb 20, 2026

The casing of the directory is incorrect (unit instead of Unit). I guess that's hard to fix on Windows. I'll take care of it.

Ah. Directory names aren't case sensitive in Windows. I guess lowercase unit somehow ended up in the git mv part of the commit.

While we're renaming the trait anyway, I'd prefer we don't use the Test prefix for it. This prefix should only be used for testcase classes, not for traits. (Also, this will allow us to have a DeclarationListTest in the future if we want it.)

I think you mean suffix. I'm open to alternative suggestions - DeclarationListTester?

@oliverklee
Copy link
Copy Markdown
Collaborator

I think you mean suffix. I'm open to alternative suggestions - DeclarationListTester?

Ah, yes, of course suffix.

I‘d be equally fine with DeclarationListTester as well as SharedDeclarationListTests.

@JakeQZ
Copy link
Copy Markdown
Collaborator Author

JakeQZ commented Feb 20, 2026

I‘d be equally fine with DeclarationListTester as well as SharedDeclarationListTests.

Maybe DeclarationListTests - it says what it provides via use (it's not a tester itself, and that it might be shared between TestCases is perpahps not relevant to the trait itself)...

@oliverklee
Copy link
Copy Markdown
Collaborator

Then let‘s go with DeclarationListTests.

@JakeQZ JakeQZ marked this pull request as ready for review February 20, 2026 22:14
@JakeQZ JakeQZ requested a review from oliverklee February 20, 2026 22:15
@oliverklee oliverklee merged commit cb56b23 into main Feb 20, 2026
24 checks passed
@oliverklee oliverklee deleted the task/tests/declarationlist branch February 20, 2026 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

developer-specific Issues that only affect maintainers, contributors, and people submitting PRs refactor For PRs that refactor code without changing functionality testing PRs/issues adding additional tests only, or primarily testing-focused

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants