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

Tests fail on windows #122

Open
BHare1985 opened this issue May 31, 2015 · 5 comments
Open

Tests fail on windows #122

BHare1985 opened this issue May 31, 2015 · 5 comments

Comments

@BHare1985
Copy link

See this commit on what I changed to make the tests windows-friendly: BHare1985@02beb86

I ignored the zip tests, as I am not supporting zip files right now (mapbox/node-zipfile#58 ). Will apply the fix there when I find time later tonight, and fix any tests for zips.

I am currently having an issue with support.js' rm function not removing absolute.json from the copypath in util afterEach hook. rmdirSync Saying directory is not empty when trying to delete copypath (because absolute.json is still there, despite getting a unlinkSync)

Will update this issue in the next few days as I get zip working and figure out the rm issue.

@BHare1985
Copy link
Author

Got zip working. A few weird things I noticed:

This is what I had to change for tests to pass:
BHare1985@4843ea9#diff-c1129c8b045390789fa8ff62f2c6b4a9

It seems like sometimes the resolved datasource file is __dirname+cache/layers/ like it was expected, and sometimes it's __dirname+data/ and sometimes it's __dirname+tmp. Also the naming convention is a bit off. I am not sure if tests havent been updated in a while to account for what caching does or if windows is getting really confused when trying to cache.

Here you can see the JSON of what was expected (according to your current tests) vs what windows actually saw:

https://www.diffchecker.com/d6lvsj84

@springmeyer
Copy link
Member

@BHare1985 great work getting this going. Take a look at using the 'rimraf' module for deleting directories with files. Also it sure why you are seeing the dfferent cache path results. Can you create a pull request for your change as a next step? that way we will be able to see if they are passing on linux via travis.

@BHare1985
Copy link
Author

@springmeyer The issue isn't deleting directories with files, it's that it shouldn't technically have a file in the directory since it is using unlinkSync (and it fires) and then rmdirSync fires and says the thing that was just unlinkSync is there, suggesting maybe it wasn't really synchronous?

Also I am not sure what good it would be to pull request the hacked up tests that pass on windows. I am 99% confident something weird is happening on windows and caching is not correct. None the less I will make a pull request for the good changes I did but not the ones where I have to modify locations of caches. It's also worth mentioning that my intuition that cache is not working right is because my grainstore is failing tests because millstone on windows is taking too long and the 100 ms timeouts in the tests for grainstore fail because millstone takes too long.

√ correctly handles re-downloading a zip that is invalid in its cached state (1254ms)
√ correctly handles invalid json
√ correctly handles missing shapefile at relative path
√ correctly handles missing shapefile at absolute path
√ correctly handles images with no extension (234ms)
√ correctly handles mac os x zipped archives with the lame __MACOSX/ subfolder (499ms)
√ correctly localizes remote image/svg files (312ms)
√ correctly localizes zipped json (296ms)
√ correctly handles a zipfile containing multiple shapefiles without corrupting data (515ms)
√ correctly handles files without symlinking
√ correctly handles a zipfile containing multiple shapefiles without corrupting data
√ correctly detects content-disposition from kml
√ correctly detects content-disposition from google docs csv
√ correctly detects content-disposition from geoserver
√ correctly detects content-disposition from cartodb
√ correctly detects content-type bin
√ correctly detects datacouch csv content-type
√ correctly detects geonode content-types
1) correctly caches remote files
√ correctly handles datasources with uppercase extensions
√ correctly prefers (for back compatibility) shapefiles over .txt files in zip archive
isRelative
  √ detects C:\ as an absolute path on Windows
  √ detects C:\ as relative path on non-Windows
  √ detects paths starting with \ as absolute on Windows
  √ detects paths starting with \ as relative on non-Windows
  √ detects paths starting with / as absolute on non-Windows
  √ detects paths starting with / as absolute on Windows
  √ detects C:\ as it should path on current platform
  √ detects paths starting with \ as it should on current platform
  √ detects paths starting with / as absolute on all platforms

util
  √ copies all files from shapefiles (and no extras)
  √ copies single files correctly
  2) "after each" hook


31 passing (4s)
2 failing

Ill have a pull request of my good fixes to the testing

@BHare1985
Copy link
Author

Well one test failed from wget and the other failed but predictably I think. Seems you have a kind of chroot for testing and it failed that way.

Most importantly is that your correctly caches remote files tests passed, whereas on windows mine is failing horribly. I understand getting millstone working on windows is low priority. Caching is low priority for me too, as long as it's working (and seems to be doing OK so far), but if I find some time this week I will take a look into this issue if you havent already.

@BHare1985
Copy link
Author

I finally came back to this and realize the issue with unlink is basically the same one as shelljs/shelljs#49 (node links in there) TLDR; when you unlink its marked as deleted but not actually deleted by windows. Thats why I get rmdirSync

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

No branches or pull requests

2 participants