Skip to content

[TASK] Add accessor tests for OutputFormat (part 2) #862

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 1, 2025

Conversation

oliverklee
Copy link
Collaborator

Currently, this class uses magic getters and setters. These should be proper methods instead.

But first, we should have tests for the accessors. This is part 2.

Currently, this class uses magic getters and setters. These
should be proper methods instead.

But first, we should have tests for the accessors. This is
part 2.
@coveralls
Copy link

coveralls commented Feb 1, 2025

Coverage Status

coverage: 45.347%. remained the same
when pulling 34495bb on feature/output-format-accessors-2
into dcb658c on main.

Copy link
Collaborator

@JakeQZ JakeQZ left a comment

Choose a reason for hiding this comment

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

One test method is missing Initially from its name.

Also wondering about ' ' as separator test values not being particularly real-world.

*/
public function setSpaceAfterSelectorSeparatorSetsSpaceAfterSelectorSeparator(): void
{
$value = ' ';
Copy link
Collaborator

Choose a reason for hiding this comment

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

An empty string or a newline might be a more realistic (non-default) real-world example.

But that said, maybe it's better to use multiple spaces in the tests, to avoid inadvertently using the default value - though newline is not the default value for any of these properties AFAIK.

@oliverklee oliverklee requested a review from JakeQZ February 1, 2025 19:16
@JakeQZ JakeQZ merged commit 95005c9 into main Feb 1, 2025
21 checks passed
@JakeQZ JakeQZ deleted the feature/output-format-accessors-2 branch February 1, 2025 19:21
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.

3 participants