write tests that assert the markers are shown or hidden as in the new example code

publiclab / leaflet-blurred-location-display

A library to display points which have been "blurred" for privacy with leaflet-blurred-location

https://publiclab.github.io/leaflet-blurred-location-display/examples/index.html

GNU General Public License v3.0

11 stars 12 forks source link

write tests that assert the markers are shown or hidden as in the new example code #52

Closed sagarpreet-chadha closed 5 years ago

sagarpreet-chadha commented 5 years ago

Fixes #45 !

sagarpreet-chadha commented 5 years ago

Hi @jywarren !

dist/Leaflet.BlurredLocationDisplay.js is not being loaded before the tests are loaded . Can you help in setting up the testing environment ? Thanks !

sagarpreet-chadha commented 5 years ago

Hi @jywarren , Kindly take a look at it once 😄 !

sagarpreet-chadha commented 5 years ago

Yes tried that as well ! Added LBL , LBLD dependencies in vendor , still showing the same error . LBLD is not getting loaded before the specs starts running .

jywarren commented 5 years ago

Hmm, confusing. Maybe you have to include the source files like this?

https://github.com/publiclab/PublicLab.Editor/blob/master/Gruntfile.js#L44-L46

If this doesn't work, maybe ping @rexagod or someone else from image-sequencer as they are really good with test setup!

sagarpreet-chadha commented 5 years ago

Hey @rexagod , @tech4GT , @publiclab/reviewers ! Can someone with good testing background help me solve this one ? Looking forward to hear from you guys :smile: . Thank you :)

rexagod commented 5 years ago

Noticing that Travis and local both give different errors (BlurredLocation and BlurredLocationDisplay respectively). @sagarpreet-chadha Are you getting an asynchronous warning in your logs as well?

Working on this.

rexagod commented 5 years ago

Hello, @sagarpreet-chadha! I've made some comments above, depending on some checks I made below.

Screenshot from 2019-03-22 23-51-19

PS. All the tests are passing if length is expected to be 0, meaning that for some reason the DOMchildren aren't being loaded.

rexagod commented 5 years ago

Getting contradicting results.

Screenshot from 2019-03-23 17-43-01

Screenshot from 2019-03-23 17-42-47

rexagod commented 5 years ago

Made some changes along the way.

https://github.com/publiclab/leaflet-blurred-location-display/blob/246b4da88847ccb7f8fc6e29d943911bfacd8dc1/src/ui/gridCenterRectangle.js#L43

Changed to if(map && map.getZoom() >= 3 && map.getZoom() <=9), since getZoom wasn't initialized onDOMload so we might want to look into that as well.

https://github.com/publiclab/leaflet-blurred-location-display/blob/246b4da88847ccb7f8fc6e29d943911bfacd8dc1/src/ui/gridCenterRectangle.js#L35

if(typeof options.blurredLocation.getRectangle !== "undefined"), removed parenthesis from above, since getRectangle itself wasn't on blurredLocation's chain.

sagarpreet-chadha commented 5 years ago

Hi @rexagod ...thank you for the recommendations 😄 . What do you think could be the possible reason that LBL and LBLD libraries are not getting loaded before testing starts ? Thanks !

sagarpreet-chadha commented 5 years ago

Thank you @arpansac , the fixtures are now being loaded 🎉 . 2 out of 5 tests are running now 👍 .

Also now all libraries are also getting loaded except LBLD .

Now we just have to figure why LBLD is not getting loaded . Thanks all 😄 !

sagarpreet-chadha commented 5 years ago

We use Jasmine jquery to load fixtures , but this package is not maintained . Also see this which i found in its documentation (this should solve the error which one gets when grunt jasmine is run on localhost) : Source : https://github.com/velesin/jasmine-jquery

And this error on Travis 😄 asks to open issue in puppeteer package :

sagarpreet-chadha commented 5 years ago

grunt-contrib-Jasmine runs on phantomJS , and phantomJS does not support ES6 !

sagarpreet-chadha commented 5 years ago

YAYYY 🎉

I think i will write a blog about it :P . So some ES6 functions were causing problem because phantomJS does not support ES6 ! And regarding LBLD not loading in fixtures --- it is yet an unsolved issue in Jasmine-jquery !

@jywarren ...kindly review 😄 !
Should we add put this library to npm ?

sagarpreet-chadha commented 5 years ago

@jywarren ...kindly review this one as well 😄 !

jywarren commented 5 years ago

OK! Sorry i had replied via email but it didn't go through!

jywarren commented 5 years ago

Awesome! Thanks Sagarpreet and great problem solving, this was such a tough one. I'm sorry i wasn't able to help more! Perhaps a good deal of this testing workflow is starting to age a bit. But you got it! 🙌