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 failing test for filterBy #12622

Closed
wants to merge 1 commit into from
Closed

Conversation

csantero
Copy link
Contributor

DO NOT MERGE

I believe this test demonstrates behavior that used to work. The first deepEqual assertion passes but the second one fails.

This is related to #12475.

This is a duplicate of #12538, which got messed up because it was targeting release.

@fivetanley
Copy link
Member

@csantero I'm trying to use this test to do a git bisect, but it appears to pass on the latest release and canary. The only failures on Travis look to be styleguide related. Does that seem right?

@csantero
Copy link
Contributor Author

csantero commented Jan 6, 2016

@fivetanley No that doesn't make sense, it was definitely failing last time I looked it. I'll take a look.

@csantero
Copy link
Contributor Author

csantero commented Jan 6, 2016

@fivetanley I've rebased off master and fixed the JSCS error in the test. The expected failure is appearing on travis.

obj = EmberObject.extend({
a: filterBy('array', 'foo')
}).create({
array: [a1, a2]
Copy link
Contributor

Choose a reason for hiding this comment

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

The test fails because we do not use prototype extensions by default in the test suite, so this use of a plain array is not supported. If you switch it to Ember.A([a1, a2]), the test passes.

@ef4
Copy link
Contributor

ef4 commented Jan 18, 2016

This test passes when prototype extensions are enabled, so I don't think it's a real reproduction of the issue in #12475

@ef4 ef4 closed this Jan 18, 2016
@csantero
Copy link
Contributor Author

@ef4 I see, thanks.

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