-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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 colorsEqualEpsilon to ModelSpec test #7078
Conversation
Thanks for the pull request @OmarShehata!
Reviewers, don't forget to make sure that:
I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome. 🌍 🌎 🌏 |
@OmarShehata you should be able to do |
We have a custom jasmine matcher that should automatically call Color.equalsEpsilon when you do |
Ah, didn't realize that already existed. Thanks for pointing that out @hpinkos ! This seems to work. |
Thanks @OmarShehata! The tests pass locally now. There was a weird test failure in the travis build though. I just restarted it to make sure it wasn't related to this change. I'll merge this if that passes |
@OmarShehata I'm not sure exactly why, but the change you made is causing all of the model tests to fail in travis. I'm guessing something isn't being cleaned up correctly, so after your tests runs it's causing You can reproduce this locally by running |
Specs/Scene/ModelSpec.js
Outdated
|
||
expect(scene).toRenderAndCall(function(rgba) { | ||
expect(rgba).toEqualEpsilon([169, 3, 3, 255], 5); // Red | ||
primitives.remove(m); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this needs to be moved outside the function block?
I think you were right @hpinkos ! |
Works great @OmarShehata, thanks! |
This fixes the test failure in #7073.
I figured it would be useful to be able to test if colors are close enough so our tests are more flexible. Feedback is welcome!
I don't think the color change is due to #6877 because it looks like the
scene
in the spec doesn't have a globe initialized, so I assume that means it's not rendering the atmosphere. The box itself still looks fine in CesiumJS.