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

Expression tests should check "serialized" #6253

Closed
jfirebaugh opened this issue Mar 1, 2018 · 1 comment
Closed

Expression tests should check "serialized" #6253

jfirebaugh opened this issue Mar 1, 2018 · 1 comment
Labels
GL native → GL JS For feature parity with Mapbox Maps SDK on a native platform testing 💯

Comments

@jfirebaugh
Copy link
Contributor

#6181 added functionality to the expression tests to verify that the actual "serialized" representation matches an expected value. However, it omitted a serialization mechanism in the JS implementation; the JS test harness ignores the expected value for existing tests, and bootstraps newly added tests with an "expected" serialization that's copied from the input expression.

This introduces a new source of friction in the gl-js/gl-native workflow: it's easy to create a new expression test in gl-js that fails unexpectedly when pulled into gl-native because the serialized representation is not exactly equivalent to the input expression. When this happens, you have to either ignore that test in gl-native (even if it would otherwise be passing), or pause your current work and update the expected serialization in a gl-js PR.

/cc @ChrisLoer

@anandthakker
Copy link
Contributor

Yeah -- it wouldn't be too hard to add Expression#serialize() to GL JS, but as of now, we'd be doing it solely for the purpose of these tests. Still, it's not that much code to add, so it's probably worth doing.

@anandthakker anandthakker added the GL native → GL JS For feature parity with Mapbox Maps SDK on a native platform label Mar 30, 2018
anandthakker pushed a commit that referenced this issue Apr 6, 2018
anandthakker pushed a commit that referenced this issue Apr 6, 2018
anandthakker added a commit that referenced this issue Apr 6, 2018
* prettify expression test diff output

* Implement Expression#serialize()

* Enable GL JS serialization and roundtripping tests

Closes #6253

* Show serialized form in test output
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GL native → GL JS For feature parity with Mapbox Maps SDK on a native platform testing 💯
Projects
None yet
Development

No branches or pull requests

3 participants