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

[5.4] Remove duplicated test for __call() #16346

Closed
wants to merge 1 commit into from
Closed

[5.4] Remove duplicated test for __call() #16346

wants to merge 1 commit into from

Conversation

mathieutu
Copy link
Contributor

@mathieutu mathieutu commented Nov 10, 2016

There wasn't any modification of the __call method with the HigherOrderCollectionMapFromArrays addition (#16274), so we don't need to test it again.
In my mind the removed test was just a vestige of a copy/paste.

There wasn't any modification of the `__call` method with the `HigherOrderCollectionMapFromArrays` addition, so we don't need to test it again.
@mathieutu mathieutu changed the title Remove duplicated test for __call() [5.4] Remove duplicated test for __call() Nov 10, 2016
@GrahamCampbell
Copy link
Member

👎 Just because a test executes the same code, it doesn't make it a duplicate. This tests uppercasing works. Think about it as a blackbox test.

@mathieutu
Copy link
Contributor Author

Yeah but this is doing exactly the same thing and this doesn't test at all the new feature: the method is about testing an array, theses lines test the behavior of an object. We can't really talk about perfs here but for me it's just dead code. It's as you want, it was just to remove a (in my mind) useless copy/paste.

@taylorotwell
Copy link
Member

I don't see this as a duplicated test. There is behavior that could regress if this test was removed.

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