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 unit tests for the point feature. #684

Merged
merged 1 commit into from
Mar 30, 2017
Merged

Add unit tests for the point feature. #684

merged 1 commit into from
Mar 30, 2017

Conversation

manthey
Copy link
Contributor

@manthey manthey commented Mar 30, 2017

Fixed some issues with the pointFeature:

  • Setting the position on construction resulted in different behavior than setting it afterwards.
  • Setting the clustering on construction allowed non-default values, but during the setter would only allow defaults.
  • If set, the clustering could not be changed except by unsetting it and resetting it.
  • The maximum radius of any point is used when deciding where to search for points, but it was potentially not calculated until after it was used.

Made some improvements:

  • The position function wraps the position in another function to handle clustering. If the position is not a function, then only one function call is made rather than two.
  • Removed the unused internal _boundingBox function from the pointFeature.
  • Added some comments about when you would want to use a different primitiveShape with the gl point feature.

@manthey
Copy link
Contributor Author

manthey commented Mar 30, 2017

I think this PR illustrates why we want to have unit tests on everything -- I discovered a few bugs and inconsistencies while writing them that weren't exposed in our various test cases.

Fixed some issues with the pointFeature:
- Setting the position on construction resulted in different behavior than setting it afterwards.
- Setting the clustering on construction allowed non-default values, but during the setter would only allow defaults.
- If set, the clustering could not be changed except by unsetting it and resetting it.
- The maximum radius of any point is used when deciding where to search for points, but it was potentially not calculated until after it was used.

Made some improvements:
- The position function wraps the position in another function to handle clustering.  If the position is not a function, then only one function call is made rather than two.
- Removed the unused internal _boundingBox function from the pointFeature.
- Added some comments about when you would want to use a different primitiveShape with the gl point feature.
@codecov-io
Copy link

codecov-io commented Mar 30, 2017

Codecov Report

Merging #684 into master will increase coverage by 1.86%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #684      +/-   ##
==========================================
+ Coverage   92.02%   93.89%   +1.86%     
==========================================
  Files          84       84              
  Lines        8857     8856       -1     
==========================================
+ Hits         8151     8315     +164     
+ Misses        706      541     -165
Impacted Files Coverage Δ
src/pointFeature.js 100% <100%> (+42.06%) ⬆️
src/gl/pointFeature.js 100% <100%> (+7.29%) ⬆️
src/util/wigglemaps.js 49.77% <0%> (+40.35%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 31ddb36...03fea0f. Read the comment docs.

@manthey manthey merged commit cf4ca12 into master Mar 30, 2017
@manthey manthey deleted the test-points branch March 30, 2017 14:29
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