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

Fix asserts in test #94

Merged
merged 1 commit into from
Feb 6, 2016
Merged

Conversation

jakemac53
Copy link
Contributor

A statement like 1 < 2 < 3 does not do what it appears at first glance ;). It actually compares 1 < 2, which returns true, casts that to a number (which gives 1), and then compares 1 < 3. This just splits up the asserts to do what I think was intended.

@jakemac53
Copy link
Contributor Author

Heh, now the tests are actually failing as well :)

@jakemac53
Copy link
Contributor Author

Looks like its assert.isTrue(giant.height <= 50); and assert.isTrue(giant.width <= 50); which are failing, I don't know where the 50 number came from or if its important, please advise me if you want the check to just change to a larger number (I get 58 as the actual value) or remove it entirely or what.

@bicknellr
Copy link
Contributor

Can you rebase this? We just added box-sizing: border-box;, which I think the tester originally assumed; maybe it works now?

@bicknellr
Copy link
Contributor

Whoops, actually we added box-sizing: content-box; - ignore my last comment. (Except for the rebase part.)

A statement like `1 < 2 < 3` does not do what it appears at first glance ;). It actually compares `1 < 2`, which returns `true`, and then continues on its way. This just splits up the asserts to do what I think was intended.
@jakemac53
Copy link
Contributor Author

Ok I rebased, but it looks like the test is still failing. IMO this should still be merged even though it will turn the bots red, because it is a legitimate failure (its only green now because the test is broken...).

@notwaldorf
Copy link
Contributor

Makes sense! Thanks for tracking this down!

notwaldorf added a commit that referenced this pull request Feb 6, 2016
@notwaldorf notwaldorf merged commit af0a8d4 into PolymerElements:master Feb 6, 2016
@bicknellr bicknellr mentioned this pull request Feb 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants