Skip to content
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

Add test for the $hidden property #2687

Merged
merged 5 commits into from Nov 30, 2023

Conversation

Treggats
Copy link
Contributor

@Treggats Treggats commented Nov 26, 2023

As requested in #1119

closes #1119

This adds a test which covers the working of the $hidden property.

Copy link
Member

@alcaeus alcaeus left a comment

Choose a reason for hiding this comment

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

Just a minor suggestion for consistency, but LGTM other than that.

tests/PropertyTest.php Outdated Show resolved Hide resolved
* @method static Builder truncate()
* @method static Eloquent sole(...$parameters)
*/
final class Animal extends Eloquent
Copy link
Member

Choose a reason for hiding this comment

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

I don't see what is tested with this model class. Hidden fields and the toArray feature are tested with HiddenAnimal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea of the two classes is that one has the hidden property and another doesn't. As to prove the working of the hidden property.

Or do you suggest to have one class and use different fields to test?

Copy link
Member

Choose a reason for hiding this comment

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

IMHO, the test on HiddenAnimal already covers the feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough, will remove this class and its usage

tests/PropertyTest.php Outdated Show resolved Hide resolved
Co-authored-by: Andreas Braun <alcaeus@users.noreply.github.com>
tests/PropertyTest.php Outdated Show resolved Hide resolved
tests/PropertyTest.php Outdated Show resolved Hide resolved
tests/PropertyTest.php Outdated Show resolved Hide resolved
tests/PropertyTest.php Outdated Show resolved Hide resolved
@GromNaN GromNaN merged commit 1fb3e9e into mongodb:4.1 Nov 30, 2023
15 checks passed
@GromNaN
Copy link
Member

GromNaN commented Nov 30, 2023

Thank you @Treggats

@Treggats Treggats deleted the feature/1119-add-addition-tests branch November 30, 2023 11:00
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.

Does laravel-mongodb respect $hidden?
3 participants