Skip to content
This repository has been archived by the owner on Jun 10, 2021. It is now read-only.

Some empty selector sets pass the previous guard clause #5

Closed
wants to merge 1 commit into from
Closed

Some empty selector sets pass the previous guard clause #5

wants to merge 1 commit into from

Conversation

spicycode
Copy link

For example, we were seeing a selector of "" that didn't get caught, and subsequently raised an exception on down the line.

For example, we were seeing a selector of "" that didn't get caught, and
subsequently raised an exception on down the line.
@spicycode
Copy link
Author

cc: @leongersing

@josh
Copy link
Owner

josh commented Dec 16, 2013

Could you add a failing test? These lines should function the same.

@spicycode
Copy link
Author

@josh We couldn't get anything to work re: running it locally (boxen says everything is kosher, but it didn't work). This change was a result of chasing down a failure inside of gist, and we've been unable to recreate it outside of gist.

@josh
Copy link
Owner

josh commented Dec 17, 2013

Maybe you aren't running the latest version. That line was only added 4 days ago 54dab87.

@spicycode
Copy link
Author

That was the line in question that we had to patch, see https://github.com/josh/selector-set/pull/5/files#diff-5d3757768563b0c40226d91384aedcdfR308

@spicycode
Copy link
Author

I'm trying to create a test to recreate the failure, as well as retrying the update that exposed the issue in the app.

@josh
Copy link
Owner

josh commented Dec 17, 2013

The only way this change could make a difference is if this.selectors is anything other than an Array. In that case, it would be a symptom of a different bug.

@spicycode spicycode closed this Dec 17, 2013
@spicycode spicycode deleted the fix-small-coersion-bug-in-query-all branch December 17, 2013 16:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants