PaddeK / node-maxmind-db

This is the pure Node API for reading MaxMind DB files. MaxMind DB is a binary file format that stores data indexed by IP address subnets (IPv4 or IPv6).
GNU Lesser General Public License v2.1
88 stars 25 forks source link

Improve Tests using tapejs #17

Closed rajeshsegu closed 9 years ago

rajeshsegu commented 9 years ago

I hated the tests in this library as they used just colors to know if a test is passed or failed. I have implemented tapejs tests for this module to make sure we can integrate them with any ci to validate every commit that goes into this module.

now you can run:

npm run test

1..26 tests 26 pass 26 ok

npm run coverage

---------------------- ---------- ---------- ---------- ---------- ---------------- File % Stmts % Branch % Funcs % Lines Uncovered Lines
node-maxmind-db/ 95.45 100 85.71 95.45
index.js 95.45 100 85.71 95.45 39
node-maxmind-db/lib/ 87.42 74.36 80.33 88.4
Decoder.js 86.78 71.01 85.71 88.3 ... 260,301,318
IPParser.js 100 100 100 100
Metadata.js 82.86 100 50 82.86 ... 28,32,36,40
Reader.js 85.37 72.22 86.67 86.42 ... 132,134,154
---------------------- ---------- ---------- ---------- ---------- ----------------
All files 87.93 75.21 80.88 88.86
---------------------- ---------- ---------- ---------- ---------- ----------------
rajeshsegu commented 9 years ago

Also, is there a reason why there is 4 space indent at places and 2 space indent at other places ? I like the 2 space indent better. If we agree on something we can make this more uniform. Thanks.

rajeshsegu commented 9 years ago

@EaterOfCode @PaddeK any thing more that you guys want me to fix ?

the-eater commented 9 years ago

sorry for the late reply, my personal favor is on 4 spaces as the original code was 4 space indent. I may have ruined it by mixing tabs with it. the code I wrote for this project is extremely bad. (Im sorry @PaddeK). the only thing that really still needs to be fixed is caching as lookups take alot of time.

I have made a patch for adding path lookup and ipv4 start node caching but never found the motivation afterwards to finish it: here.

rajeshsegu commented 9 years ago

@EaterOfCode @PaddeK Please do review

rajeshsegu commented 9 years ago

@EaterOfCode @PaddeK lets merge this if you guys do not have more review comments. Thanks in advance.